Joomla JDatabase: selectRowNumber strange use of class attribute or method

Question

in Joomla 3.9.16 there is a strage use of a class attribute that is also used as a method!

In libraries/joomla/database/query.php you can find:

protected $selectRowNumber = null;

but also

public function selectRowNumber($orderBy, $orderColumnAlias)
{
  $this->validateRowNumber($orderBy, $orderColumnAlias);
  $this->select("ROW_NUMBER() OVER (ORDER BY $orderBy) AS $orderColumnAlias");

  return $this;
}

You can undestand why reading this method of the same class:

protected function validateRowNumber($orderBy, $orderColumnAlias)
{
  if ($this->selectRowNumber)
  {
    throw new RuntimeException("Method 'selectRowNumber' can be called only once per instance.");
  }

  $this->type = 'select';

  $this->selectRowNumber = array(
    'orderBy' => $orderBy,
    'orderColumnAlias' => $orderColumnAlias,
  );
}

When you call $this->selectRowNumber() as a method for the first time, it calls $this->validateRowNumber() in which $this->selectRowNumber() becomes an array! If you call again $this->selectRowNumber() it throws an exception because you can call it only once and is no more a method.

I wonder if this is a good programming practice, I think definitely NOT because is not easy to understand and mantain. Maybe you can achive the same result in another way much more clear and linear. What I ask is: am I right or this is a common practice?

Thanks

Answer 1

Are you saying that you can reproduce this fault? I am going to assume that you are misreading the script.

It looks to me that the selectRowNumber() method is called the first time and then inside validateRowNumber(), the class variable/property $this->selectRowNumber is checked for truthiness.

Since it is null (falsey) the first time, no exception is thrown.

The class variable/property (not the method) is updated with

$this->selectRowNumber = array(
    'orderBy' => $orderBy,
    'orderColumnAlias' => $orderColumnAlias,
  );

Then back inside selectRowNumber(), the ROW_NUMBER() OVER (ORDER BY $orderBy) AS $orderColumnAlias string is applied to $this->select().

Because ROW_NUMBER() OVER (ORDER BY $orderBy) AS $orderColumnAlias must not be applied twice to a given query, the safeguard / thrown Exception checks for a truthy class variable/property -- which of course it is because it has been modified from null to a non-empty associative array.

Is it confusing to have properties and methods sharing the same name? Sure. The only distinction between the two is the trailing ().

Is this acceptable coding practice? Well, in terms of readability it's not ideal. But once a project standardizes its naming convention, sometimes these kinds of converging names happen. The bigger the code base, the more likely this is to occur. I think consistency is more important than readability in this isolated situation.