Page 2 of 3

Re: Amusing or annoying code

Posted: Mon Dec 06, 2010 10:32 pm
by Dr Danyo
Ok I'll bite,

What value is assigned to Min_Negative ? :)

Rather than checking the size of the collection, isn't it simpler to just call Collection.isEmpty ? At least that way you never have to worry about someone changing the value of Min_Positive.

Dr Danyo.

Re: Amusing or annoying code

Posted: Tue Dec 07, 2010 2:01 pm
by BeeJay
we have got a global constant called "Min_Positive" which is defined as a Real equal to 0.
One could argue that Min_Positive is a somewhat misleading name for the global constant, given that zero isn't a actually a positive number?!? ;)

I'm with Dr D. on this one, using Collection.isEmpty makes the code more readable as well, even if you have to put in a not statement:

Code: Select all

if not collection.isEmpty ... endif;
... assuming of course the only code within the if statement isn't something like:

Code: Select all

if not collection.isEmpty foreach obj in collection ... endforeach; endif;
Cheers,
BeeJay.

Re: Amusing or annoying code

Posted: Wed Dec 08, 2010 8:23 am
by allistar
[

Code: Select all

if not collection.isEmpty foreach obj in collection ... endforeach; endif;
Eek - two trips to the server to get two locks instead of one (assuming you're not in load state).

Even worse is when you see code like this (the collection is an exclusive member collection):

Code: Select all

if (allThingies <> null) then foreach thingy in allThingies do endforeach; endif;

Re: Amusing or annoying code

Posted: Wed Dec 08, 2010 8:37 am
by BeeJay
Eek - two trips to the server to get two locks instead of one (assuming you're not in load state).
... and assuming you're not in lock state (yuck!) and that it's not a stable or frozen collection. ;)

Cheers,
BeeJay.

Re: Amusing or annoying code

Posted: Thu Dec 09, 2010 4:45 am
by Rich Cassell
I have seen on a few occasions methods which return a boolean actually set an entirely separate global boolean...

Something like...

Code: Select all

doValidate() : Boolean; vars begin if something then globalBool := true; else globalBool := false; endif; return true; end;
The calling method then uses this boolean to continue:

Code: Select all

doValidate(); //Call doValidate if globalBool then // Continue endif; end;

Re: Amusing or annoying code

Posted: Fri Dec 10, 2010 12:25 pm
by Stokes
Ok I'll bite,

What value is assigned to Min_Negative ? :)

Rather than checking the size of the collection, isn't it simpler to just call Collection.isEmpty ? At least that way you never have to worry about someone changing the value of Min_Positive.

Dr Danyo.
There is only Min_Positive in our system. Here is the comment that was made in 1997 why it was implemented:

"Added, constant for checking that various pieces of data are greater than or equal to zero when we are maintaining them.This is to prevent the use of actual numbers in code."

I always use isEmpty unless we are using a collection that stores active and inactive data, then I will use collection.sizeOfActive > 0
we have got a global constant called "Min_Positive" which is defined as a Real equal to 0.
One could argue that Min_Positive is a somewhat misleading name for the global constant, given that zero isn't a actually a positive number?!? ;)

I'm with Dr D. on this one, using Collection.isEmpty makes the code more readable as well, even if you have to put in a not statement:

Code: Select all

if not collection.isEmpty ... endif;
... assuming of course the only code within the if statement isn't something like:

Code: Select all

if not collection.isEmpty foreach obj in collection ... endforeach; endif;
Cheers,
BeeJay.
Good point! It is good to know I have your support on doing a global search and replace on Min_Positive and getting rid of it once and for all.

Re: Amusing or annoying code

Posted: Fri Dec 10, 2010 2:32 pm
by murray
There is only Min_Positive in our system. Here is the comment that was made in 1997 why it was implemented:
"Added, constant for checking that various pieces of data are greater than or equal to zero when we are maintaining them.This is to prevent the use of actual numbers in code."
Heh, sounds like blind adherence to a dogma. If you dont like the hard-coded 0 you can always use the null keyword.
However (and I am being sarcastic here), it did leave you the opportunity of redefining zero if you so wished! ;)

Re: Amusing or annoying code

Posted: Fri Dec 10, 2010 5:23 pm
by ghosttie
I always use isEmpty unless we are using a collection that stores active and inactive data, then I will use collection.sizeOfActive > 0
I hope sizeOfActive doesn't loop through the collection counting them. You might be better off using conditions to create an active and inactive collection.

Re: Amusing or annoying code

Posted: Fri Dec 10, 2010 9:05 pm
by Dr Danyo
Good point! It is good to know I have your support on doing a global search and replace on Min_Positive and getting rid of it once and for all.
Yeah get rid of it, there is a time and a place for replacing literals (magic numbers) with constants, the key to this, is the literal has to have a specific meaning, e.g DRINKING_AGE, TAX_RATE, make good constants for 18 and 12.5.

Min_Positive doesnt have a specific enough meaning, its too vague and actually makes the code less readable IMHO, than just using null or 0.

see http://www.refactoring.com/catalog/repl ... stant.html for some more dogma. :D

Dr Danyo.

Re: Amusing or annoying code

Posted: Tue Dec 14, 2010 1:12 pm
by BeeJay
I find the following example of an unnecessary use of io on a method parameter somewhat annoying as well - probably down to people not properly understanding the various parameter usage types:

Code: Select all

doSomething( pSomeCollection : MyCollClass io ); vars someObject : MyClass ; begin someObject := ...; pSomeCollection.add( someObject ); end;
ie: Too many people as soon as they get "cannot update constant" go and add io to the parameter definition.

In the case of the above quasi code they should actually have added input not io, as per the following quasi code, since they are not changing the collection to which the pSomeCollection parameter refers.

Code: Select all

doSomething( pSomeCollection : MyCollClass input ); vars someObject : MyClass ; begin someObject := ...; pSomeCollection.add( someObject ); end;
Cheers,
BeeJay.