Nov 21, 2014

Your code tells about you more than you think

Suppose you have the following in your code:

$this->pid_list = $this->pid_list ? $this->pid_list : 0;
if (! $this->pid_list) {
$this->errors[] = 'No pid_list defined';
}

This translates to: “If the value is not true, than set it to false. If the value is false, set the error message”. As you see, there is some room for improvement here: there is no need to set the value to false if it is not true because it is already false.

When you code, think what you write. The code above tells that the author either does not know PHP well enough or he is not thinking clearly.

2 comments:

  1. ?
    No, I disagree. What if $this->pid_list is '' (empty string) or an empty array at the beginning. In both cases it will be 0 at the end, which may be desired.

    ReplyDelete
  2. $this->pid_list is string. The code later does not use $this->pid_list but displays errors. So:

    if (!$this->pid_list) {
    $this->errors[] = 'No pid_list defined';
    }

    would be fully enough.

    ReplyDelete