Best Coding Practice

First, let me say that this question is a rather general programming
question, but the context is PHP, so I figured this group would have
the most relevant insight.

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.
#1.
public function addArrayA($object){
//logic
$a[] = $object;
}

public function addArrayB($object){
//same logic
$b[] = $object;
}


#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;
}
}

or

#3
public function addArray($object, $arr = "a"){
//logic
$$arr[] = $object;
}



I ended up going with option number 1, because I felt that despite the
inefficient, redundant code it would later be more readable to other
programmers that might work on the project. Additionally, I didn't
feel wholly comfortable with default variables being the only
difference between a full price product and a free product. Thoughts?

burgermeister01@gmail.com wrote:
> First, let me say that this question is a rather general programming
> question, but the context is PHP, so I figured this group would have
> the most relevant insight.
>
> 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.
> #1.
> public function addArrayA($object){
> //logic
> $a[] = $object;
> }
>
> public function addArrayB($object){
> //same logic
> $b[] = $object;
> }
>
>
> #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;
> }
> }
>
> or
>
> #3
> public function addArray($object, $arr = "a"){
> //logic
> $$arr[] = $object;
> }
>
>
>
> I ended up going with option number 1, because I felt that despite the
> inefficient, redundant code it would later be more readable to other
> programmers that might work on the project. Additionally, I didn't
> feel wholly comfortable with default variables being the only
> difference between a full price product and a free product. Thoughts?
>

I'd pick choice 1.

Generally I try not to change an existing function's signature unless
necessary. Also, it's more clear as to what it's doing, and there is
only one line of duplicated code (if there were a lot of lines, I'd have
a common function and pass $a or $b to that function).

You could make a case could be made for choice 2, but it adds
complication (and processing time for the if statements).

No way would I ever pick choice 3. It's unnecessarily complicated and
confusing. And what would happen if someone passed 'c' to it?

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

On Aug 23, 8:41 pm, "burgermeiste...@gmail.com"
<burgermeiste...@gmail.com> wrote:
> First, let me say that this question is a rather general programming
> question, but the context is PHP, so I figured this group would have
> the most relevant insight.
>
> 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.
> #1.
> public function addArrayA($object){
> //logic
> $a[] = $object;
>
> }
>
> public function addArrayB($object){
> //same logic
> $b[] = $object;
>
> }

still have redundant logic, as you stated

>
> #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.

>
> or
>
> #3
> public function addArray($object, $arr = "a"){
> //logic
> $$arr[] = $object;
>
> }
>

I've actually done something similar to this before. Only 2 months
later I was cursing the programmer that came up with this
chaos....damn.

> I ended up going with option number 1, because I felt that despite the
> inefficient, redundant code it would later be more readable to other
> programmers that might work on the project. Additionally, I didn't
> feel wholly comfortable with default variables being the only
> difference between a full price product and a free product. Thoughts?

<burgermeister01@gmail.com> wrote in message
news:1187916107.291436.304670@q3g2000prf.googlegro ups.com...
| First, let me say that this question is a rather general programming
| question, but the context is PHP, so I figured this group would have
| the most relevant insight.

since this is a general prog. q., let me answer with a formal approach...

i'd get more strict with the array. i'd make them into an object - a class.
i'd either have b extend a, or a and b extend another base/common object, or
have a and b implement a standard inter face. the array that you're holding
$object in should be strong-typed as the 1) base object or 2) the shared
interface. upon processing within your function, you'd simply get the class
type of the object and place it in its appropriate array.

but, that just may be to formal...especially since isp's seem reluctant/slow
in upgrading to more recent and oop compatible versions of php.

and, that could just be me.

cheers.

ELINTPimp wrote:
> On Aug 23, 8:41 pm, "burgermeiste...@gmail.com"

>> 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.

Refactoring?
What's that?

"Sanders Kaufman" <bucky@kaufman.net> wrote in message
news:CMrzi.11327$3x.7315@newssvr25.news.prodigy.ne t...
> ELINTPimp wrote:

>> not very good practice. when you start using conditional statements
>> within your code, it's a good canidate for refactoring.
>
> Refactoring?
> What's that?

http://www.google.com.au/search?q=refactoring

Second hit.

--
Richard.

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

>> Refactoring?
>> What's that?
>
> http://www.google.com.au/search?q=refactoring

http://en.wikipedia.org/wiki/Refactoring

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. Oddly, my spell-checker sees "refactor" as not-a-word.

I'll have to remember that one - I don't rewrite my code, I
refactor(sp?) it.

I notice they cite Agile and Extreme (but not Waterfall) in that
definition. That's funny. Every time I've been involved in a shop that
prayed from one of those bibles, agony and failure were the
words-of-the-day... for just that reason.

They always come up with convoluted ways of stating the simplest things,
and the most complex ways of performing the simplest functions.

On second thought... maybe I'll just forget that one.

Thx rf

"Sanders Kaufman" <bucky@kaufman.net> wrote in message
news:nHszi.4651$LL7.2735@nlpi069.nbdc.sbc.com...
| rf wrote:
| > "Sanders Kaufman" <bucky@kaufman.net> wrote in message
|
| >> Refactoring?
| >> What's that?
| >
| > http://www.google.com.au/search?q=refactoring
|
| http://en.wikipedia.org/wiki/Refactoring
|
| 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. Oddly, my spell-checker sees "refactor" as not-a-word.
|
| I'll have to remember that one - I don't rewrite my code, I
| refactor(sp?) it.
|
| I notice they cite Agile and Extreme (but not Waterfall) in that
| definition. That's funny. Every time I've been involved in a shop that
| prayed from one of those bibles, agony and failure were the
| words-of-the-day... for just that reason.
|
| They always come up with convoluted ways of stating the simplest things,
| and the most complex ways of performing the simplest functions.
|
| On second thought... maybe I'll just forget that one.

but don't you just love the 'automatic unit testing blah blah blah'. i've
written tests more complex that the code it was performed on. perhaps
'automatic' should be stricken from that def.

however, the 'fundamentally wrong' is not really wrong at all. the
functionality is not broken. it could be that a new enhancement needs to be
added. refactoring old code for the addition may just mean code isolation so
that pertinent portions can be used in multiple places yet maintained in
one. it may mean you hired a consultant to produce quick results that you
were understaffed to complete. *always* go behind your consultants!!! *most*
of the shittiest working code i've ever seen comes from that source. plus
they may just innocently enough, not know your standards and practices...so
you're bringing it back in line. it could be that when reviewing a code
base, patterns emerge that could be consolidated or reduced to a more simple
statement...all of these things are reasons to refactor.

btw, i hate 'extreme programming'. it employs decades old scientific
management models and pawns them off as new and exciting, even
revolutionary...dare i say 'extreme'...methodology not seen until modern
times. but, i digress...

cheers

"Sanders Kaufman" <bucky@kaufman.net> wrote in message
news:nHszi.4651$LL7.2735@nlpi069.nbdc.sbc.com...
> rf wrote:
>> "Sanders Kaufman" <bucky@kaufman.net> wrote in message
>
>>> Refactoring?
>>> What's that?
>>
>> http://www.google.com.au/search?q=refactoring
>
> http://en.wikipedia.org/wiki/Refactoring
>
> 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.

--
Richard.

> > 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.