Best Coding Practice

rf wrote:
> "Sanders Kaufman" <bucky@kaufman.net> wrote in message

>> Oh, I get it. It's a politically-correct, bidness-safe way of saying you
>> did something fundamentally wrong and have to re-design the whole damned
>> thing.
>
> No it is most definately not. If you had read and understood the above
> article you would know that.
>
> While you are looking again at the article you may wish to follow the link
> to Extreme Programming, of which refactoring is an integral component.
>
> This is not, of course, programming 101. It's many levels above that.

Yeah - so advanced that it leaves the programming world entirely, and
enters that of PC bidness-speak. Bleccchh.

I'm a code-monkey, and I like it.

burgermeister01@gmail.com wrote:

> I needed to add an array class member to an object. It was exactly the
> same as another class member, except that one array stored regular
> products, and the other stored free promotional products. As such, I
> needed a way to add products to the new, free array. Since all the logic
> was the same between the two arrays aside from the price, I had a few
> different options to do this.

Frankly I think all the options you outlined expose too much of the inner
workings of your class. What happens when you add a third category of
products (products that cost money, free products & products we have to
pay you to take away!)

A better solution would be something like this:

public function add_product (Product $p)
{
if ($p->price==0)
$this->free_products[] = $p;
else
$this->products[] = $p;
}

--
Toby A Inkster BSc (Hons) ARCS
[Geek of HTML/SQL/Perl/PHP/Python/Apache/Linux]
[OS: Linux 2.6.12-12mdksmp, up 64 days, 11:44.]

TrivialEncoder/0.2
http://tobyinkster.co.uk/blog/2007/0...ivial-encoder/

burgermeister01@gmail.com wrote:
>>> Anyways, this is also more of an opinion based question than one
>>> seeking a definite answer. Recently, while maintaining a rather large
>>> system. I needed to add an array class member to an object. It was
>>> exactly the same as another class member, except that one array stored
>>> regular products, and the other stored free promotional products. As
>>> such, I needed a way to add products to the new, free array. Since all
>>> the logic was the same between the two arrays aside from the price, I
>>> had a few different options to do this. I'm interested in polling
>>> which way some of the group members feel would have been the best.
>>> Naturally these are abbreviated versions of what I envisioned as
>>> possible solutions.
>>> #2. (These next two are arranged as such, because the class using
>>> these functions is included in many script files,
>>> all of which I may not be aware of, so there would have to be some
>>> default value for the array that was always used
>>> before this new array was needed)
>>> public function addArray($object, $free = NULL){
>>> //logic
>>> if(!$free){
>>> $a[] = $object;
>>> }else{
>>> $b[] = $object;
>>> }
>>> }
>> not very good practice. when you start using conditional statements
>> within your code, it's a good canidate for refactoring. Seeing this
>> as a possibility, I probably would have created a abstract base
>> Products class, which it's child concrete classes would act as
>> containers for product items. The base class would contain all the
>> core functionality, and the child classes the specific elements.
>> Then, come time to add a new sibling class, it simply inherets from
>> the superclass and your good to go.
>>
>
>
>
> Frankly, I can't believe that I didn't think of a more OO approach to
> begin with. I've been working in a shop that went through a series of
> really terrible programmers, and me, wanting to stick to convention
> just for consistency's sake, it appears I have begun to lose some of
> my 'good' habits. I will have to consult this group more frequently to
> ensure that doesn't happen any further. In fact, this code is so fresh
> I may still go back and rewrite it.
>
> That being the case, I have some more specific inquires into the
> group's and ElINT's opinion on OOP. It's possible I'm not fully
> understanding the way you describe it, ELINT, so if I'm reiterating
> what you've already said, I apologize in advance. The approach I might
> take, is as follows:
> My Product class which is a member of the Order class, of which I
> mentioned in my OP, would be my parent class. My reasoning for this is
> because the Product class is already used in so many other scripts, I
> wouldn't feel entirely comfortable suddenly making it a child class of
> some other super class. Additionally, the data used to instantiate the
> Product class is being pulled from a separate database table (like an
> active record) however, the free products essentially are the items
> from the Product table, only with their price overridden to $0 (that
> in itself may not make a lot of sense, but apparently, after
> communicating with the client its the best way to interface with their
> inventory system). Therefore, in my mind it would make sense from an
> OO standpoint to make a FreeProduct class which extends the Product
> class and simply overrides the price member, or the function that
> returns it.
> Further thoughts and opinions on this matter are welcome and
> appreciated.
>

Why would you have a FreeProduct class? All a FreeProduct is is a
Product with a cost of 0. Cost is an attribute, and different values
for attributes should not determine new products. Would you have a
ProductCostsNinetyNineCents class? Just set the cost to zero, instead
of overriding it.

Or is there something I'm missing?

--
==================
Remove the "x" from my email address
Jerry Stuckle
JDS Computer Training Corp.
jstucklex@attglobal.net
==================

On Aug 24, 12:59 am, Sanders Kaufman <bu...@kaufman.net> wrote:
> rf wrote:
> > "Sanders Kaufman" <bu...@kaufman.net> wrote in message
> >> Oh, I get it. It's a politically-correct, bidness-safe way of saying you
> >> did something fundamentally wrong and have to re-design the whole damned
> >> thing.
>
> > No it is most definately not. If you had read and understood the above
> > article you would know that.
>
> > While you are looking again at the article you may wish to follow the link
> > to Extreme Programming, of which refactoring is an integral component.
>
> > This is not, of course, programming 101. It's many levels above that.
>
> Yeah - so advanced that it leaves the programming world entirely, and
> enters that of PC bidness-speak. Bleccchh.
>
> I'm a code-monkey, and I like it.

Sanders, I respect where you are coming from as wanting to be a "code-
monkey" and remove yourself from the "PC bidness-speak", as I've been
that way (and continue to be that way to some extent). There is a big
difference, however, in adopting sound best practices in software
engineering and "running with the crowd", as it were, using terms we
all love to hate like Web 2.0 and the like.

Refactoring your code doesn't always mean you screwed up in the design
or within the actual logic of the code. Most of the time, it has to
deal with improving your code to meet new business requirements. A
good (and dramatic) example is if you started with a very small
project that used OO, but not an MVC or similar framework. Later,
your customer tells you that their business has expanded and their
presence on the Internet must grow accordingly and now the customer is
needing something that is well beyond the original specifications of
the web application you created. Then starts the refactoring
process. Refactoring could also deal not just with your business
logic, but also database schema and aspects therein.

I recommend to all that hasn't already read them, to check out books
by Martin Fowler as a starter into this subject.

The term "refactoring" can be considered a label, really, similar to
giving a design pattern a name. It gives technical people a common
language so they can work together. Just because your PHB may pickup
on the term, and use it (probably incorrectly), doesn't mean it's
garbage and does not relate to a software engineer.

Extreme programming uses the term refactoring, but even though it may
be in the index of the book, doesn't mean they are strictly related.

On Aug 24, 6:17 am, Jerry Stuckle <jstuck...@attglobal.net> wrote:
> burgermeiste...@gmail.com wrote:
> >>> Anyways, this is also more of an opinion based question than one
> >>> seeking a definite answer. Recently, while maintaining a rather large
> >>> system. I needed to add an array class member to an object. It was
> >>> exactly the same as another class member, except that one array stored
> >>> regular products, and the other stored free promotional products. As
> >>> such, I needed a way to add products to the new, free array. Since all
> >>> the logic was the same between the two arrays aside from the price, I
> >>> had a few different options to do this. I'm interested in polling
> >>> which way some of the group members feel would have been the best.
> >>> Naturally these are abbreviated versions of what I envisioned as
> >>> possible solutions.
> >>> #2. (These next two are arranged as such, because the class using
> >>> these functions is included in many script files,
> >>> all of which I may not be aware of, so there would have to be some
> >>> default value for the array that was always used
> >>> before this new array was needed)
> >>> public function addArray($object, $free = NULL){
> >>> //logic
> >>> if(!$free){
> >>> $a[] = $object;
> >>> }else{
> >>> $b[] = $object;
> >>> }
> >>> }
> >> not very good practice. when you start using conditional statements
> >> within your code, it's a good canidate for refactoring. Seeing this
> >> as a possibility, I probably would have created a abstract base
> >> Products class, which it's child concrete classes would act as
> >> containers for product items. The base class would contain all the
> >> core functionality, and the child classes the specific elements.
> >> Then, come time to add a new sibling class, it simply inherets from
> >> the superclass and your good to go.
>
> > Frankly, I can't believe that I didn't think of a more OO approach to
> > begin with. I've been working in a shop that went through a series of
> > really terrible programmers, and me, wanting to stick to convention
> > just for consistency's sake, it appears I have begun to lose some of
> > my 'good' habits. I will have to consult this group more frequently to
> > ensure that doesn't happen any further. In fact, this code is so fresh
> > I may still go back and rewrite it.
>
> > That being the case, I have some more specific inquires into the
> > group's and ElINT's opinion on OOP. It's possible I'm not fully
> > understanding the way you describe it, ELINT, so if I'm reiterating
> > what you've already said, I apologize in advance. The approach I might
> > take, is as follows:
> > My Product class which is a member of the Order class, of which I
> > mentioned in my OP, would be my parent class. My reasoning for this is
> > because the Product class is already used in so many other scripts, I
> > wouldn't feel entirely comfortable suddenly making it a child class of
> > some other super class. Additionally, the data used to instantiate the
> > Product class is being pulled from a separate database table (like an
> > active record) however, the free products essentially are the items
> > from the Product table, only with their price overridden to $0 (that
> > in itself may not make a lot of sense, but apparently, after
> > communicating with the client its the best way to interface with their
> > inventory system). Therefore, in my mind it would make sense from an
> > OO standpoint to make a FreeProduct class which extends the Product
> > class and simply overrides the price member, or the function that
> > returns it.
> > Further thoughts and opinions on this matter are welcome and
> > appreciated.
>
> Why would you have a FreeProduct class? All a FreeProduct is is a
> Product with a cost of 0. Cost is an attribute, and different values
> for attributes should not determine new products. Would you have a
> ProductCostsNinetyNineCents class? Just set the cost to zero, instead
> of overriding it.
>
> Or is there something I'm missing?
>
> --
> ==================
> Remove the "x" from my email address
> Jerry Stuckle
> JDS Computer Training Corp.
> jstuck...@attglobal.net
> ==================

Jerry,
Initially I was going to answer your post with a description about how
Product classes are active records and FreeProducts aren't *really*
active records. However, the more I thought about it, the more I
realized that that's in fact no justification for a child class in
this case. In fact, because of the way the Order class stores
products, i.e. basically $products['product_id_number'] = $quantity,
this would only add an extra layer of complexity and not really make
storing the products any easier. This leads me back to what you and I
originally concurred about being the better solution (or something
like it, such as passing in an array, as you suggested, or denoting
the appropriate array by product characteristics as per Toby).
To others this may not sound optimal, but this system is rather large,
and has become rather convoluted over time, and as such it's hard to
describe the different factors I need to account for; I really do
think that an OO approach would be overcomplicated in this case. I'm
sure that by this point I sound a bit like a crazy-man that can't make
up his mind, but I think for me this is been more of lesson in how
difficult it can be to keep code in your head over time and be able to
talk with others about it intelligently.

On Aug 24, 7:17 am, Jerry Stuckle <jstuck...@attglobal.net> wrote:
> burgermeiste...@gmail.com wrote:
> >>> Anyways, this is also more of an opinion based question than one
> >>> seeking a definite answer. Recently, while maintaining a rather large
> >>> system. I needed to add an array class member to an object. It was
> >>> exactly the same as another class member, except that one array stored
> >>> regular products, and the other stored free promotional products. As
> >>> such, I needed a way to add products to the new, free array. Since all
> >>> the logic was the same between the two arrays aside from the price, I
> >>> had a few different options to do this. I'm interested in polling
> >>> which way some of the group members feel would have been the best.
> >>> Naturally these are abbreviated versions of what I envisioned as
> >>> possible solutions.
> >>> #2. (These next two are arranged as such, because the class using
> >>> these functions is included in many script files,
> >>> all of which I may not be aware of, so there would have to be some
> >>> default value for the array that was always used
> >>> before this new array was needed)
> >>> public function addArray($object, $free = NULL){
> >>> //logic
> >>> if(!$free){
> >>> $a[] = $object;
> >>> }else{
> >>> $b[] = $object;
> >>> }
> >>> }
> >> not very good practice. when you start using conditional statements
> >> within your code, it's a good canidate for refactoring. Seeing this
> >> as a possibility, I probably would have created a abstract base
> >> Products class, which it's child concrete classes would act as
> >> containers for product items. The base class would contain all the
> >> core functionality, and the child classes the specific elements.
> >> Then, come time to add a new sibling class, it simply inherets from
> >> the superclass and your good to go.
>
> > Frankly, I can't believe that I didn't think of a more OO approach to
> > begin with. I've been working in a shop that went through a series of
> > really terrible programmers, and me, wanting to stick to convention
> > just for consistency's sake, it appears I have begun to lose some of
> > my 'good' habits. I will have to consult this group more frequently to
> > ensure that doesn't happen any further. In fact, this code is so fresh
> > I may still go back and rewrite it.
>
> > That being the case, I have some more specific inquires into the
> > group's and ElINT's opinion on OOP. It's possible I'm not fully
> > understanding the way you describe it, ELINT, so if I'm reiterating
> > what you've already said, I apologize in advance. The approach I might
> > take, is as follows:
> > My Product class which is a member of the Order class, of which I
> > mentioned in my OP, would be my parent class. My reasoning for this is
> > because the Product class is already used in so many other scripts, I
> > wouldn't feel entirely comfortable suddenly making it a child class of
> > some other super class. Additionally, the data used to instantiate the
> > Product class is being pulled from a separate database table (like an
> > active record) however, the free products essentially are the items
> > from the Product table, only with their price overridden to $0 (that
> > in itself may not make a lot of sense, but apparently, after
> > communicating with the client its the best way to interface with their
> > inventory system). Therefore, in my mind it would make sense from an
> > OO standpoint to make a FreeProduct class which extends the Product
> > class and simply overrides the price member, or the function that
> > returns it.
> > Further thoughts and opinions on this matter are welcome and
> > appreciated.
>
> Why would you have a FreeProduct class? All a FreeProduct is is a
> Product with a cost of 0. Cost is an attribute, and different values
> for attributes should not determine new products. Would you have a
> ProductCostsNinetyNineCents class? Just set the cost to zero, instead
> of overriding it.
>
> Or is there something I'm missing?
>
> --
> ==================
> Remove the "x" from my email address
> Jerry Stuckle
> JDS Computer Training Corp.
> jstuck...@attglobal.net
> ==================

Actually, Jerry makes a good point. I guess I was just looking at
what you were doing rather than really analyzing WHAT you were trying
to accomplish.

IF this is all your trying to accomplish (discounting or rendering a
product "free"), a separate class would be incorrect. Thanks for
pointing that out, Jerry.

I think we need to step back and look at what you are doing a bit.
You have a Products class that is a child of the order class. Not
sure of the reasoning behind that from what you wrote, but I'll roll
with it. Your products class currently have two arrays, one for
standard products and one for free products. Why do these need to be
broken out into two arrays? I wonder about the db schema on how a
product is identified as "free". Is there a bit flag or something of
the sort, or maybe a discount column in percentage where 100% ==
free? Possibly something like this:

class product { //
private $_product_id;
private $_price;
private $_discount = 0;
private $_free_flag = false;
private $_quantity;
...

static public function getProduct($product_id) {
//returns the product by product ID, called from the PRODUCTS
class
//this can ensure you only have the product listed once in your
array.
//if it already exists, just adjust the quantity
}

public function isFree() {
//checks to see if the product is free
if ($this->_free_flag === true || $this->_discount == 100) {
return true;
}
return false;
}
} // end product class

class products {
$_products = array();

public function getFreeItems() {
//go through the array checking if the item is free
return $freeItems;
}

} // end products class

On Aug 24, 10:20 am, "burgermeiste...@gmail.com"
<burgermeiste...@gmail.com> wrote:
> On Aug 24, 6:17 am, Jerry Stuckle <jstuck...@attglobal.net> wrote:
>
>
>
> > burgermeiste...@gmail.com wrote:
> > >>> Anyways, this is also more of an opinion based question than one
> > >>> seeking a definite answer. Recently, while maintaining a rather large
> > >>> system. I needed to add an array class member to an object. It was
> > >>> exactly the same as another class member, except that one array stored
> > >>> regular products, and the other stored free promotional products. As
> > >>> such, I needed a way to add products to the new, free array. Since all
> > >>> the logic was the same between the two arrays aside from the price, I
> > >>> had a few different options to do this. I'm interested in polling
> > >>> which way some of the group members feel would have been the best.
> > >>> Naturally these are abbreviated versions of what I envisioned as
> > >>> possible solutions.
> > >>> #2. (These next two are arranged as such, because the class using
> > >>> these functions is included in many script files,
> > >>> all of which I may not be aware of, so there would have to be some
> > >>> default value for the array that was always used
> > >>> before this new array was needed)
> > >>> public function addArray($object, $free = NULL){
> > >>> //logic
> > >>> if(!$free){
> > >>> $a[] = $object;
> > >>> }else{
> > >>> $b[] = $object;
> > >>> }
> > >>> }
> > >> not very good practice. when you start using conditional statements
> > >> within your code, it's a good canidate for refactoring. Seeing this
> > >> as a possibility, I probably would have created a abstract base
> > >> Products class, which it's child concrete classes would act as
> > >> containers for product items. The base class would contain all the
> > >> core functionality, and the child classes the specific elements.
> > >> Then, come time to add a new sibling class, it simply inherets from
> > >> the superclass and your good to go.
>
> > > Frankly, I can't believe that I didn't think of a more OO approach to
> > > begin with. I've been working in a shop that went through a series of
> > > really terrible programmers, and me, wanting to stick to convention
> > > just for consistency's sake, it appears I have begun to lose some of
> > > my 'good' habits. I will have to consult this group more frequently to
> > > ensure that doesn't happen any further. In fact, this code is so fresh
> > > I may still go back and rewrite it.
>
> > > That being the case, I have some more specific inquires into the
> > > group's and ElINT's opinion on OOP. It's possible I'm not fully
> > > understanding the way you describe it, ELINT, so if I'm reiterating
> > > what you've already said, I apologize in advance. The approach I might
> > > take, is as follows:
> > > My Product class which is a member of the Order class, of which I
> > > mentioned in my OP, would be my parent class. My reasoning for this is
> > > because the Product class is already used in so many other scripts, I
> > > wouldn't feel entirely comfortable suddenly making it a child class of
> > > some other super class. Additionally, the data used to instantiate the
> > > Product class is being pulled from a separate database table (like an
> > > active record) however, the free products essentially are the items
> > > from the Product table, only with their price overridden to $0 (that
> > > in itself may not make a lot of sense, but apparently, after
> > > communicating with the client its the best way to interface with their
> > > inventory system). Therefore, in my mind it would make sense from an
> > > OO standpoint to make a FreeProduct class which extends the Product
> > > class and simply overrides the price member, or the function that
> > > returns it.
> > > Further thoughts and opinions on this matter are welcome and
> > > appreciated.
>
> > Why would you have a FreeProduct class? All a FreeProduct is is a
> > Product with a cost of 0. Cost is an attribute, and different values
> > for attributes should not determine new products. Would you have a
> > ProductCostsNinetyNineCents class? Just set the cost to zero, instead
> > of overriding it.
>
> > Or is there something I'm missing?
>
> > --
> > ==================
> > Remove the "x" from my email address
> > Jerry Stuckle
> > JDS Computer Training Corp.
> > jstuck...@attglobal.net
> > ==================
>
> Jerry,
> Initially I was going to answer your post with a description about how
> Product classes are active records and FreeProducts aren't *really*
> active records. However, the more I thought about it, the more I
> realized that that's in fact no justification for a child class in
> this case. In fact, because of the way the Order class stores
> products, i.e. basically $products['product_id_number'] = $quantity,
> this would only add an extra layer of complexity and not really make
> storing the products any easier. This leads me back to what you and I
> originally concurred about being the better solution (or something
> like it, such as passing in an array, as you suggested, or denoting
> the appropriate array by product characteristics as per Toby).
> To others this may not sound optimal, but this system is rather large,
> and has become rather convoluted over time, and as such it's hard to
> describe the different factors I need to account for; I really do
> think that an OO approach would be overcomplicated in this case. I'm
> sure that by this point I sound a bit like a crazy-man that can't make
> up his mind, but I think for me this is been more of lesson in how
> difficult it can be to keep code in your head over time and be able to
> talk with others about it intelligently.

Agreed, please see my post above. I'll be sure to focus on the
problem rather than the approach of the person in the future. Thanks
for pointing this out, Jerry.

| That being the case, I have some more specific inquires into the
| group's and ElINT's opinion on OOP. It's possible I'm not fully
| understanding the way you describe it, ELINT, so if I'm reiterating
| what you've already said, I apologize in advance. The approach I might
| take, is as follows:
| My Product class which is a member of the Order class, of which I
| mentioned in my OP, would be my parent class. My reasoning for this is
| because the Product class is already used in so many other scripts, I
| wouldn't feel entirely comfortable suddenly making it a child class of
| some other super class.

i *completely* disagree...you are not programming for your *comfort*. you
program to get it correctly. you should have an *item* class implemented by
product which will further be extended and used by the free product class.
additionally - and the main reason you should *not* make the base class
'order' - your company could begin selling *services* and as far as billing
goes, would be so similar in structure as 'product', you'd want to reuse the
base that way. finally, since an 'order' consists of line 'items', you can
use the 'item' interface to process the order (shipping, billing, inventory,
etc.) without having to look at any other specific differences between the
inheritors (product, free product, service).

again, do it right so the code base is *sound*...don't avoid doing something
because it is *uncomfortable*. the only reason to keep from refactoring
completely is because of costs.

but that's just my 0.02 usd

| Initially I was going to answer your post with a description about how
| Product classes are active records and FreeProducts aren't *really*
| active records. However, the more I thought about it, the more I
| realized that that's in fact no justification for a child class in
| this case. In fact, because of the way the Order class stores
| products, i.e. basically $products['product_id_number'] = $quantity,
| this would only add an extra layer of complexity and not really make
| storing the products any easier. This leads me back to what you and I
| originally concurred about being the better solution (or something
| like it, such as passing in an array, as you suggested, or denoting
| the appropriate array by product characteristics as per Toby).
| To others this may not sound optimal, but this system is rather large,
| and has become rather convoluted over time, and as such it's hard to
| describe the different factors I need to account for; I really do
| think that an OO approach would be overcomplicated in this case.

stop right there! statements like that are said by people that DON'T
understand OOP. they say it so frequently that all but seasoned developers
believe there is truth to it. that propogates MORE people give birth to
millions of lines of shitty code because they shy away from OOP and what it
REALLY offers.

it should be BECAUSE your system is becoming convoluted that you should use
OOP!!! otherwise, you'll end up with a monolith that will eventually topple.

| Agreed, please see my post above. I'll be sure to focus on the
| problem rather than the approach of the person in the future. Thanks
| for pointing this out, Jerry.

you always have to consider BOTH. that 'person in the future' is probably
going to be you. either way, it is the structure and pattern of what you
code that makes a project manageable and expandable. just looking at the
current problem is amatuer practice at best. that's called putting out fires
and is too short-sighted to do anyone much good.