Yii2: This loop does not loop

[s1lver]

Hi!

I look at the sentence (#16151), the change in the definition of the method modifier and I understand that something is wrong in the body of the method itself. Although I can be wrong. Correct me if not.

This loop will execute only once. Then why is there a loop?

foreach ($this->from as $alias => $tableName) {
    if (is_string($alias)) {
        return [$tableName, $alias];
    }
    break;
}

I'm watching the story, and I see that the php-fixer was applied with the rule of condition reduction.

Old condition

foreach ($this->from as $alias => $tableName) {
    if (is_string($alias)) {
        return [$tableName, $alias];
    } else {
        break;
    }
}

It also moved to version 3.0. Can from a loopget rid?

Additional info

| Q | A
| ---------------- | ---
| Yii version | 2.0.15.1
| PHP version | 7.2.0
| Operating system | Windows 10, Red Hat

[tomlutzenberger]

This code looks suspicious. Did you you find some info or hint the commit message of that change?

[s1lver]

/**
 * Returns the table name and the table alias for [[modelClass]].
 * @return array the table name and the table alias.
 * @internal
 */
protected function getTableNameAndAlias()
{
    if (empty($this->from)) {
        $tableName = $this->getPrimaryTableName();
    } else {
        $tableName = '';
        foreach ($this->from as $alias => $tableName) {
            if (is_string($alias)) {
                return [$tableName, $alias];
            }
            break;
        }
    }

    if (preg_match('/^(.*?)\s+({{\w+}}|\w+)$/', $tableName, $matches)) {
        $alias = $matches[2];
    } else {
        $alias = $tableName;
    }

    return [$tableName, $alias];
}

Commit message: Enable no_useless_else rule in php-cs-fixer (#14420)
Commit hash: fe8a0a6a2ed09ae5b255e7f3b3e16c16d8789e3a

[tomlutzenberger]

I think this needs to be refactored.

[rob006]

Commit message: Enable no_useless_else rule in php-cs-fixer (#14420)
Commit hash: fe8a0a6

This commit did not changed code behavior. Original foreach is since the beginning of framework: https://github.com/yiisoft/yii2/commit/aaa4e429a4c745999dc4c2863c5c553f50e83fbe

I guess it is some cryptic way of getting first element from array.

[cebe]

it could be replaced with

if (!empty($this->from)) {
    $tableName = reset($this->from);
    $alias = key($this->from);
    if (is_string($alias)) {
        return [$tableName, $alias];
    }
}

not sure if that makes it better.

[tomlutzenberger]

@cebe What if the first element key is numeric?

How about this:

if (!empty($this->from)) {
    $result = $this->from;
    array_walk($result, function($value, $key) {
        if (is_string($key)) {
            return [$value, $key];
        }
    });
    if (count($result)) {
        return $result[0];
    }
}

This filters all elements with a string as key and you can return the first result element.

[rob006]

What if the first element key is numeric?

Then it does not have an alias. That is how original code worked.

Anyway, I'm only for adding some short comment which explains what is happening. I don't think it is worth rewriting it, and definitely not worth changing code behavior.

[cebe]

I added a comment, hope it's clear now.