Don't add unnecessary conditions to Boolean expressions

[Ursego]

Link to this posting

by Ursego » 19 Feb 2013, 22:26

Don't add to Boolean expressions any conditions which don't change the produced result.

Foolish advice, you say? Why to add them? I cannot answer this question, but some coders do add them (they are very mysterious!) - this advice is caused by my multi-years code inspection practice. Look at the following nice examples:

Code: Select all

if (not IsNull(ld_expire_date)) and ld_expire_date < ld_today then

is logically the same as

Code: Select all

if ld_expire_date < ld_today then

because when ld_expire_date contains null then the second expression will produce null, which will be interpreted by code branching operators as false.

Code: Select all

if dw_xxx.RowCount() > 0 and dw_xxx.GetSelectedRow(0) > 0 then

must be re-written as

Code: Select all

if dw_xxx.GetSelectedRow(0) > 0 then

because when GetSelectedRow(0) returns a positive number, RowCount() has no chance to return 0 (so, why to call it?).

And now - my loved example of stupid code:

Code: Select all

if (not IsNull(ids_xxx)) and IsValid(ids_xxx) then

It is absolutely the same as

Code: Select all

if IsValid(ids_xxx) then

because if IsValid returns true (i.e. memory is allocated for the pointed object) then, of course, the variable is pointing something, so IsNull has no chance to return false. Probably, some developers think, that if a pointer is not instantiated, then IsValid will result in Null Object Reference, but no worries - it will simply return false.

Another genius piece, found my me not once (maybe, the salary of those programmers depends on the number of written lines of code?):

Code: Select all

if ll_row_count > 0 then
       for ll_row = 1 to ll_row_count
              [...]
       next
end if

It works exactly in the same way as simply

Code: Select all

for ll_row = 1 to ll_row_count
       [...]
next