Usage of legacy if/else flow control statements

objectscriptQuality release 
1.3.6
Id 
OS0089
Rule type 
Code Smell
Severity 

Critical

Critical
SQALE characteristic 
  • Changeability
    • Logic
Tags 
coding-guidelines, deprecation
Remediation function 
Constant/issue
Remediation cost 
5min

You should avoid legacy versions of if and else, and use the most modern format which uses curly braces. Below is explained the reason.

ObjectScript has several control flow statements with constructs as if, else, for, while and do..while. But there also exist legacy versions of if, else and for. The main difference is that the legacy versions don't use curly braces to separate the flow control from the code to execute; for instance:

// Newer version
if condition {
    do ..firstThing()
    do ..secondThing()
}
// Older version
if condition do ..firstThing() do ..secondThing()

It is possible to associate one codition with several statements, but all them need to be on the same line. This is the first reason why such statements should be avoided (clarity); another reason is that legacy versions can lead to several traps.

Trap #1: statements not on the same line

If you come from another C-style language, you may be used to write:

if condition
    do ..something()

However, this will not do what you think it does; as mentioned above, if you are using a legacy control flow statement, the statements executed if the condition is met need to be on the same line.

What the code above essentially means is that while the if statement is executed and the condition evaluated, no statements are associated to it. Therefore, the code above will execute do ..something() regardless of whether the condition is met.

Trap #2: implicit evaluation of $TEST

This is particularly evident in the code sample below:

if condition set $test = 0
else  write "condition not met" // Note the two spaces between "else" and "write"

If you try and execute a method/routine with the code above, regardless of whether the condition evaluates to true, the output will always be:

condition not met

The problem here is that legacy else does not require to be paired with a legacy if; it is just a command which will execute its associated statement(s) (if any, see above) if the value of the special $TEST variable evaluates to boolean true.

And as you will see if you read the $TEST documentation, this command may be set by commands you wouldn't suspect use it at a first glance. One example:

// Lock global foo; print a warning if locking fails
lock +foo else  write "failed to lock foo" // again, note the two spaces! 

And the confusion does not end here, look at this:

// Do something if $TEST evaluates to boolean true.
// And note that if is here abbreviated to a single i
i  do ..something()