Removed non-default call of $app-get() in favor of using $app->has()

[SamMousa]

Q	A
Is bugfix?	no
New feature?	no
Breaks BC?	no
Tests pass?	yes

This is a small code quality improvement. Related to #14184

[lynicidn]

this equal:
return $user->getId();
https://github.com/yiisoft/yii2/blob/master/framework/web/User.php#L338

[SamMousa]

Nice, even better :)

[SamMousa]

I think this can be merged.

[samdark]

!$user->isGuest check is missing. Because of that behavior is not the same as it was.

[SamMousa]

Actually this is already part of the getter:

    public function getId()
    {
        $identity = $this->getIdentity();

        return $identity !== null ? $identity->getId() : null;
    }

    public function getIsGuest()
    {
        return $this->getIdentity() === null;
    }

[samdark]

Changed code doesn't look like equivalent of the previous code.

[cebe]

why do you think it is better to call two methods instead of one? Or what exactly is the improvement? I see no reason to change it.

[SamMousa]

@cebe Depending on the implementation there could be large differences.

If the API offers a has and a get then for checking it makes sense to use has. There could be an implementation where checking is a lot cheaper than actually retrieving.

Why even offer has if it is better to just check the result of get?

[samdark]

has() is there for cases when you don't need the value but actually need to check if there's something or not.

[samdark]

@SamMousa how about #14185 (comment)?

[SamMousa]

@samdark See the comment I just added.
@cebe Using $app->has does not instantiate the component; $app->get() does.

If your code does this:

if ($app->get('test', false) === null) {
    // Some stuff here.
    $app->get('test')->someFunc();
    $app->get('someOtherComponent')->test();
}

It doesn't matter what you use. But if then, 2 years later you don't need the 3rd line and you remove it:

if ($app->get('test', false) === null) {
    // Some stuff here.
    $app->get('someOtherComponent')->test();
}

Now you're instantiating a component you no longer need; you just need to know it exists.

[cebe]

But if then, 2 years later you don't need the 3rd line

I am fine with changing it to has() two years later then ;)

[SamMousa]

But who says it is actually @cebe making the change? The point I was trying to make is that the code is not descriptive.
You're not getting a component you're checking if the application knows about a component.
It doesn't matter if you get it or not 2 lines later, in the guard of the if statement you are just checking existence.

[dynasource]

it's better readable, although it is less obvious that guests return null

[samdark]

Merged. Thanks!