Buy
Buy

Optional type-hinting & Semantic Methods

I need to show you something - so start another battle between some Jedi Star Fighters. It works... but if I refresh enough times... come on... yes! It blows up!

Argument 2 passed to BattleResult::__construct() must be an instance
of Ship, null given.

In BattleResult - because we're good programmers - we type-hinted the two Ship arguments. Buuuuut, if you look at the battle() function, there's a case where the ships can destroy each other. And when that happens, there is no winning or losing ship - they're both null. Since - news flash null is not a Ship object, PHP gets angry and casts down this big error.

When you type-hint an argument, the value must be that class - not even null is ok. But sometimes you do have a spot where an argument might be a specific object, or it might be null. To support this, make the argument optional - add an = null after it:

... lines 1 - 2
class BattleResult
{
... lines 5 - 13
public function __construct($usedJediPowers, Ship $winningShip = null, Ship $losingShip = null)
{
$this->usedJediPowers = $usedJediPowers;
$this->winningShip = $winningShip;
$this->losingShip = $losingShip;
}
... lines 20 - 53
}

I don't have to, but I'll update @return on the methods to be Ship|null:

... lines 1 - 2
class BattleResult
{
... lines 5 - 36
/**
* @return Ship|null
*/
public function getLosingShip()
{
return $this->losingShip;
}
/**
* Was there a winner? Or did everybody die :(
*
* @return bool
*/
public function isThereAWinner()
{
return $this->getWinningShip() !== null;
}
}

PhpStorm will still give me auto-completion - but this is a signal to other developers not to blindly call this method and always assume it will return a Ship object. We're already coding safely in battle.php: we check to make sure getWinningShip() returns something before calling a method on it. Cool.

Adding a Semantic isThereAWinner Method

To check if a BattleResult has a winner, you can see if getWinningShip() returns null. But we can do even better. Go to BattleResult and make a new public method called isThereAWinner(). Here, return $this->getWinningShip != null:

... lines 1 - 2
class BattleResult
{
... lines 5 - 43
/**
* Was there a winner? Or did everybody die :(
*
* @return bool
*/
public function isThereAWinner()
{
return $this->getWinningShip() !== null;
}
}

There's at least two great things about this. First, code outside of this class doesn't need to know how to figure out whether or not there was a winner: that code can be dumb and just call this method. Second, if something happens in the future and the logic used to figure out if there is a winner changes, we only need to update the code in this one spot: no need to run around the code base trying to figure out where we have the old logic for seeing if there was a winner.

Update battle.php to use this. The first if statement is really trying to figure out whether or not there was a winner. Update this to $battleResult->isThereAWinner(). Use that again right below:

99 lines battle.php
... lines 1 - 70
<?php if ($battleResult->isThereAWinner()): ?>
... line 72
<?php else: ?>
... line 74
<?php endif; ?>
... lines 76 - 77
<?php if (!$battleResult->isThereAWinner()): ?>
... line 79
<?php else: ?>
... lines 81 - 86
<?php endif; ?>
... lines 88 - 99

Go back and refresh! You'll have to trust me that if we refresh this 1000 times, it'll always work - our bug is gone - and we have a nifty new helper method in BattleResult.

Leave a comment!

  • 2019-05-21 Diego Aguiar

    Hey Serhii Ivanenko

    You should use !== every time you can, it's just safer, if you find yourself with the need of using "normal" comparison (!=), you should take a minute and find out why you need it.

    About Yoda conditions, well, personally, I don't like them but it's a standard in Symfony internals, so it's up to you if you want to adopt it or not :)

    Cheers!

  • 2019-05-20 Serhii Ivanenko

    Hi!
    Thanks for your answer! My question was about calling method vs calling property. As I said below in my reply to Abiola, when I faced a real challenge, I understood the advantage of calling the method. And I also agree with you - it depends on situation.
    PS. What about != and !== I know the difference. I should write !== and even better use Yoda conditions, I mean null !== $this->getWinningShip()

  • 2019-05-20 Serhii Ivanenko

    Thanks for the explanation! And I have to say that just recently in the process of refactoring I just encountered this situation and now I understand better that the method is preferred (yes, you are right, talking about additional logic)

  • 2019-05-20 Victor Bocharsky

    Hey Abiola,

    Thanks for sharing your thoughts on it! Makes sense as well.

    Cheers!

  • 2019-05-20 Victor Bocharsky

    Hey Serhii,

    Sorry for the late replay, we had temporary outage with Disqus comments synchronization :/

    About your question, are you asking about calling method vs calling property directly? Or are you asking about the difference between "!= null" and "!== null"?

    Well, actually, if we're talking about simple getters that just return values - I'd recommend you to access property directly, i.e. call "$this->winningShip". First of all, maybe if you would do so - you even won't need that getWinningShip() method at all? If so, that's great. But if you still call that getWinningShip() somewhere outside the class, I'd still recommend you access properties directly in class, easier for refactoring in the future I think, but it depends. So, there's no a big difference at all.

    But Abiola below also have a good point, but as I said, it depends on the case, don't think too much about it. ;)

    Cheers!

  • 2019-05-19 Abiola M

    Serhii Ivanenko I see your point. I suppose the advantage is 'future-proofing'.

    Such that in future if additional logic is ever needed before outputting the winningShip, the logic would definitely sit within getWinningShip() method and then we would not need to worry about adding this additional logic to places where winningShip might have been used directly.

  • 2019-04-26 Serhii Ivanenko

    Hi
    My question is about the isThereAWinner method. You have access to private properties inside the class. So, could you please explain why don't you use just:
    return $this->winningShip != null;
    instead of
    return $this->getWinningShip() !== null;
    ?

    I mean, what is the advantage of using method in such cases?