3
\$\begingroup\$

The code below if launched on the command line is a simple guessing game.

A random character 'a', 'b' or 'c' is chosen, and the user is prompted until they guess correctly.

It's arguable that invalid input and a win is not necessarily exceptional. Should exceptions be used in this manner?

I am not sure what's a sensible way to check for a winning situation and to terminate the game.

<?php

class ABorCException extends Exception {
    const WIN     = 'Win';
    const INVALID = 'Invalid';
}

class ABorC
{
    const CHOICES = ['a', 'b', 'c'];
    public $answer;
    public $turns;

    public function __construct($answer = null)
    {
        $this->answer = $answer ?? self::CHOICES[array_rand(self::CHOICES)];
    }

    public function turn($guess)
    {
        if(!in_array($guess, self::CHOICES))
            throw new ABorCException(ABorCException::INVALID);
        $this->turns[] = $guess;
        if($guess === $this->answer)
            throw new ABorCException(ABorCException::WIN);
    }
}

$game = new ABorC;

while (true) {
    try {
        $input = readline("Enter your guess a, b or c?\n");
        $game->turn($input);
        echo "Wrong!\n", "Guess again.\n";
    } catch (ABorCException $e) {
        if($e->getMessage()===ABorCException::INVALID) {
            echo "Invalid input.\n";
        }
        if($e->getMessage()===ABorCException::WIN) {
            echo "Correct!\n", "You won in ", count($game->turns), " valid attempts.\n";
            exit;
        }
    }
}

Sample game:

$ php aborc.php 
Enter your guess a, b or c?a
Wrong!
Guess again.
Enter your guess a, b or c?z
Invalid input.
Enter your guess a, b or c?b
Correct!
You won in 2 valid attempts.
\$\endgroup\$
7
\$\begingroup\$

You are using exceptions for flow control, and that is considered an anti-pattern.

Exceptions are meant for unhandlable conditions that would otherwise corrupt your application. Winning the game should not corrupt the application and cause undefined behavior. If it does, you are likely building a stock trading application (I'm only kidding ;-) of course).

Invalid input could certainly corrupt things or cause undefined behavior, but in this case your program should expect invalid input and handle it gracefully. Always expect the unexpected with user input.

When reading input, check that it is correct. Loop until you get correct input.

The $game-turn() method could return a "result" object, which reports back whether the game has been won, and if so who the winner is (and possibly a score). Again, handle this with a simple if statement:

$result = $game->turn($input);

if ($result->isWinningTurn()) {
    echo "Winner! {$result->winner}";
}
else {
    echo "next move";
}

If $game->turn() throws an exception, I would expect it to be an unhandled condition or something unrecoverable like an out of memory exception. As a programmer, I would be VERY surprised if winning the game crashed the program.

Now, the turn() method should throw an exception if you give it bad input. That makes sense. But you need to check for valid input before calling $game->turn($input). You likely need an isValidInput method on the game object in order to check this.

$game-turn("d"); // I expect this to throw

if ($game->isValidInput($input)) {
    $result = $game->turn($input)); // I expect this to always succeed

    if ($result.isWinningTurn()) {
        ...
    }
    else {
        ...
    }
}
else {
    echo "Bad input, try again."
}
| improve this answer | |
\$\endgroup\$
3
\$\begingroup\$

I suppose I am not convinced that your simple game actually needs to establish any exceptions for these predictable failures in flow. I agree with Greg that a win should definitely not be a throwable exception -- because it is predictable.

I like this simplistic answer to "When should you use PHP Exceptions?". Because your script is handling very predictable failures in the user's actions, adding exceptions seems like unnecessary overhead / code bloat. With some simple refactoring to return the reason the guess was wrong else no reason (guess was correct), exceptions can be cleanly avoided.

Code: (tested on https://repl.it)

class ABorC
{
    const CHOICES = ['a', 'b', 'c'];
    public $answer;
    public $validGuesses = 0;

    public function __construct(?string $answer = null)
    {
        $this->answer = in_array($this->answer, self::CHOICES)
            ? $answer
            : self::CHOICES[array_rand(self::CHOICES)];
    }

    public function wrongGuessReason(string $guess): ?string
    {
        if (!in_array($guess, self::CHOICES)) {
            return 'Invalid input';
        }
        ++$this->validGuesses;
        if ($guess === $this->answer) {
            return 'Wrong!';
        }
        return null;
    }
}

$game = new ABorC;
do {
    $reasonIncorrect = $game->wrongGuessReason(readline("Enter your guess a, b or c?\n"));
    echo $reasonIncorrect . "\n";
} while ($reasonIncorrect);
printf(
    "Correct!\nYou won after %d valid attempt%s\n",
    $game->validGuesses,
    $game->validGuesses === 1 ? '' : 's'
);
  1. Please review all of the recommendations in PSR-12. I know you like to omit curly braces in your code, but this is a violation of PSR guidelines.
  2. You could also validate that any manually loaded answer is also in the expected range and fallback to a randomized answer.
  3. I am becoming increasingly supportive of the use of printf() and sprintf() instead of messy concatenation involving variables. If you are going to use commas to concatenate in your echos, then write a space on either side of the comma so that it gets the same treatment as dot-concatenation. (...not to be confused with commas used to separate arguments.)
  4. I don't see any reason to cache the guesses in your class, since you are not printing them in the conclusion. For this reason, just use $this->validGuesses as a counter and increment it on each valid turn.
  5. As soon as the guess is right, break the loop and show the congratulations.
  6. I try to avoid single-use variables, but if you want to declare a variable to store the readline() value, that's not ridiculous because it may help coders who read your code and do not know what readline() does/returns.
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.