1

I'm creating a board game (stratego) in c++ and was wondering if it considered a poor practice to return an integer from a class method in order to determine which case in a switch statement to show to the user.

Example: In stratego, you can't attack board pieces that are part of your own army so I have a message "You cannot attack your own army" when the user tries to do so.

Same thing if a movement is performed that would result in the player jumping off the board, moving too many spaces, etc.

Each of these invalid movements has it's own unique message, but to avoid printing them from the Class.cpp file, which is where the player's moves are validated, I have the Class.cpp file returning an integer to a switch statement in the main() that it was called from. What is the most recommended way to handle how the messages get called?

class Test
{
public:
    Test()
    {

    }
    int Validate_Move(int valid)
    {
        if (valid > 0 && valid < 5)
        {
            return 1;
        }
        else if (valid > 5)
        {
            return 2;
        }
    }
};

int main()
{
    int entry;
    std::cout << "Enter move: ";
    std::cin >> entry;

    Test obj;

    switch (obj.Validate_Move(entry))
    {
    case 1:
        std::cout << "Move is valid" << std::endl;
    case 2:
        std::cout << "Move is invalid" << std::endl;
    default:
        std::cout << "Error occured" << std::endl;
    }

    return 0;
}
ecaz
  • 33
  • 1
  • 4
  • 3
    This is fine practice, but it may be better to use an `enum` instead of an `int` (or `enum class` in C++11) – dwcanillas Jun 18 '15 at 19:11
  • `bool isValid(int valid) { if (valid > 0 && valid < 5) return true; if (valid > 5) return false; std::abort(); }`, and then `Test obj; if (obj.isValid(entry)) { /*...*/ }` – Kerrek SB Jun 18 '15 at 19:12
  • Your program has undefined behaviour in general since you don't return a value from a function declared to return `int`. – Kerrek SB Jun 18 '15 at 19:13
  • The "default" case is misleading. It can never be taken. – Kerrek SB Jun 18 '15 at 19:14
  • 1
    Nice, you separate logic (data processing) and user interface (besides a warning you ignored) –  Jun 18 '15 at 19:14
  • In my actual program I have the validation method return 0 at the end to indicate that it was valid move. The code I have here is just an example – ecaz Jun 18 '15 at 20:08

1 Answers1

3

There's nothing wrong with that technique. If you want to be more explicit, you could always make an enum

class Test
{
public:
    Test() = default;

    enum EValidity {eError, eValid, eInvalid};

    EValidity Validate_Move(int valid)
    {
        if (valid > 0 && valid < 5)
        {
            return eValid;
        }
        else if (valid > 5)
        {
            return eInvalid;
        }
        else
        {
            return eError;
        }
    }
};

int main()
{
    int entry;
    std::cout << "Enter move: ";
    std::cin >> entry;

    Test obj;

    switch (obj.Validate_Move(entry))
    {
    case Test::eValid:
        std::cout << "Move is valid" << std::endl;
        break;
    case Test::eInvalid:
        std::cout << "Move is invalid" << std::endl;
        break;
    case Test::eError:
        std::cout << "Error occured" << std::endl;
        break;
    default:
        assert(false);
    }

    return 0;
}
Cory Kramer
  • 114,268
  • 16
  • 167
  • 218
  • Thanks, Ill give that a shot. Looks like it will make things a little more clear overall – ecaz Jun 18 '15 at 20:09