Jump to content

  • Log In with Google      Sign In   
  • Create Account

We're offering banner ads on our site from just $5!

1. Details HERE. 2. GDNet+ Subscriptions HERE. 3. Ad upload HERE.


#ActualKhatharr

Posted 23 December 2012 - 03:17 AM

That did make it a lot smaller and got rid of the huge switch-case.  That was the next thing. something just seemed weird have that huge switch-case and that also seemed to be repeating it's self a lot.

 

Couple questions:

1: I can change my enum to start at 1 can't I?  Just saying because that is what my menu choices start from.

 

2: Instead of returning some invalid value to have the compiler not complain (I assume it'd be from not all paths returning a value, unless I am wrong?) could it be better to just set the choice to a temporary gameMode variable and then return that temp?  I say that only because I've never seen (though I also haven't looked at a lot of peoples code) a function just return some junk value, even though that part of the code would never get executed (unless I am missing something).  Though now that I think about it, since that function should never return the junk value I guess it really wouldn't matter.

 

Thanks for all the input so far everyone.

 

1) Yes, you can define the value of any enum member like I did to set the zero there for COMPUTER_PLAYER. If you do that in this case though you'll need to remember to check that the value is non-zero when you test for validity (or use it to signal invalidity: see below). If you do decide to change the start value then that last value should probably be renamed, since it no longer represents the count of valid members.

 

2) Some people get edgy about functions that have more than one exit point. In large or complex functions this can definitely be a hazard since someone changing the function later may add something that needs to be done every call but they don't notice that there's a possible return at an earlier point. You could just as easily do this:

enum GameMode : unsigned {
  GAMEMODE_INVALID = 0,
  COMPUTER_PLAYER,
  PLAYER_COMPUTER,
  GAMEMODE_END_VALUE
};

GameMode Game::GetGameMode() {
  GameMode mode = GAMEMODE_INVALID;
  while(mode == GAMEMODE_INVALID) {
    //get user input
    GameMode gameMode = (GameMode)(ToInt(StringInput()));
    
    //handle invalid input
    if(gameMode >= GAMEMODE_ENTRY_COUNT) {
      gameMode = GAMEMODE_INVALID;
      cout << "Please enter a valid choice using the number next to your desired game mode." << endl;
      cout << "Press ENTER to try again..." << endl;
      cin.get();
      DisplayGameMode();
    }
  }

  return gameMode;
}

#8Khatharr

Posted 23 December 2012 - 03:14 AM

That did make it a lot smaller and got rid of the huge switch-case.  That was the next thing. something just seemed weird have that huge switch-case and that also seemed to be repeating it's self a lot.

 

Couple questions:

1: I can change my enum to start at 1 can't I?  Just saying because that is what my menu choices start from.

 

2: Instead of returning some invalid value to have the compiler not complain (I assume it'd be from not all paths returning a value, unless I am wrong?) could it be better to just set the choice to a temporary gameMode variable and then return that temp?  I say that only because I've never seen (though I also haven't looked at a lot of peoples code) a function just return some junk value, even though that part of the code would never get executed (unless I am missing something).  Though now that I think about it, since that function should never return the junk value I guess it really wouldn't matter.

 

Thanks for all the input so far everyone.

 

enum GameMode : unsigned {
  GAMEMODE_INVALID = 0,
  COMPUTER_PLAYER,
  PLAYER_COMPUTER,
  GAMEMODE_END_VALUE
};

GameMode Game::GetGameMode() {
  GameMode mode = GAMEMODE_INVALID;
  while(mode == GAMEMODE_INVALID) {
    //get user input
    GameMode gameMode = (GameMode)(ToInt(StringInput()));
    
    //handle invalid input
    if(gameMode >= GAMEMODE_ENTRY_COUNT) {
      gameMode = GAMEMODE_INVALID;
      cout << "Please enter a valid choice using the number next to your desired game mode." << endl;
      cout << "Press ENTER to try again..." << endl;
      cin.get();
      DisplayGameMode();
    }
  }

  return gameMode;
}

#7Khatharr

Posted 23 December 2012 - 03:13 AM

That did make it a lot smaller and got rid of the huge switch-case.  That was the next thing. something just seemed weird have that huge switch-case and that also seemed to be repeating it's self a lot.

 

Couple questions:

1: I can change my enum to start at 1 can't I?  Just saying because that is what my menu choices start from.

 

2: Instead of returning some invalid value to have the compiler not complain (I assume it'd be from not all paths returning a value, unless I am wrong?) could it be better to just set the choice to a temporary gameMode variable and then return that temp?  I say that only because I've never seen (though I also haven't looked at a lot of peoples code) a function just return some junk value, even though that part of the code would never get executed (unless I am missing something).  Though now that I think about it, since that function should never return the junk value I guess it really wouldn't matter.

 

Thanks for all the input so far everyone.

 

enum GameMode : unsigned {
  GAMEMODE_INVALID = 0,
  COMPUTER_PLAYER,
  PLAYER_COMPUTER,
  GAMEMODE_END_VALUE
};

GameMode Game::GetGameMode() {
  GameMode mode = GAMEMODE_INVALID;
  while(mode == GAMEMODE_INVALID) {
    //get user input
    GameMode gameMode = (GameMode)(ToInt(StringInput()));
    
    //handle invalid input
    if( !(gameMode < GAMEMODE_ENTRY_COUNT)) {
      gameMode = GAMEMODE_INVALID;
      cout << "Please enter a valid choice using the number next to your desired game mode." << endl;
      cout << "Press ENTER to try again..." << endl;
      cin.get();
      DisplayGameMode();
    }
  }

  return gameMode;
}

#6Khatharr

Posted 23 December 2012 - 03:08 AM

That did make it a lot smaller and got rid of the huge switch-case.  That was the next thing. something just seemed weird have that huge switch-case and that also seemed to be repeating it's self a lot.

 

Couple questions:

1: I can change my enum to start at 1 can't I?  Just saying because that is what my menu choices start from.

 

2: Instead of returning some invalid value to have the compiler not complain (I assume it'd be from not all paths returning a value, unless I am wrong?) could it be better to just set the choice to a temporary gameMode variable and then return that temp?  I say that only because I've never seen (though I also haven't looked at a lot of peoples code) a function just return some junk value, even though that part of the code would never get executed (unless I am missing something).  Though now that I think about it, since that function should never return the junk value I guess it really wouldn't matter.

 

Thanks for all the input so far everyone.

 

1) Yes, you can define the value of any enum member like I did to set the zero there for COMPUTER_PLAYER. If you do that in this case though you'll need to remember to check that the value is non-zero when you test for validity. It may be better to just subtract 1 from the input value. If you do decide to change the start value then that last value should probably be renamed to something like GAMEMODE_TERMINATOR, since it no longer represents the count of valid members.

 

2) Some people get edgy about functions that have more than one exit point. In large or complex functions this can definitely be a hazard since someone changing the function later may add something that needs to be done every call but they don't notice that there's a possible return at an earlier point. You could just as easily do this:

 

GameMode Game::GetGameMode() {
  GameMode gameMode = 0xFFFFFFFF;
  while(gameMode == 0xFFFFFFFF) {
    //get user input
    gameMode = (GameMode)(ToInt(StringInput()));
    
    //Check for validity
    if(gameMode < GAMEMODE_ENTRY_COUNT) {break;}

    //validity failed, so reset the value, show message and reloop
    gameMode = 0xFFFFFFFF;
    cout << "Please enter a valid choice using the number next to your desired game mode." << endl;
    cout << "Press ENTER to try again..." << endl;
    cin.get();
    DisplayGameMode();
  }

  return gameMode;
}

Depending on your compiler you may need to typecast when you assign an int to a GameMode typed variable. Alternatively you could use a bool to break the loop, which would may be a little easier to read.


#5Khatharr

Posted 23 December 2012 - 03:08 AM

That did make it a lot smaller and got rid of the huge switch-case.  That was the next thing. something just seemed weird have that huge switch-case and that also seemed to be repeating it's self a lot.

 

Couple questions:

1: I can change my enum to start at 1 can't I?  Just saying because that is what my menu choices start from.

 

2: Instead of returning some invalid value to have the compiler not complain (I assume it'd be from not all paths returning a value, unless I am wrong?) could it be better to just set the choice to a temporary gameMode variable and then return that temp?  I say that only because I've never seen (though I also haven't looked at a lot of peoples code) a function just return some junk value, even though that part of the code would never get executed (unless I am missing something).  Though now that I think about it, since that function should never return the junk value I guess it really wouldn't matter.

 

Thanks for all the input so far everyone.

 

1) Yes, you can define the value of any enum member like I did to set the zero there for COMPUTER_PLAYER. If you do that in this case though you'll need to remember to check that the value is non-zero when you test for validity. It may be better to just subtract 1 from the input value. If you do decide to change the start value then that last value should probably be renamed to something like GAMEMODE_TERMINATOR, since it no longer represents the count of valid members.

 

2) Some people get edgy about functions that have more than one exit point. In large or complex functions this can definitely be a hazard since someone changing the function later may add something that needs to be done every call but they don't notice that there's a possible return at an earlier point. You could just as easily do this:

 

GameMode Game::GetGameMode() {
  GameMode gameMode = 0xFFFFFFFF;
  while(gameMode == 0xFFFFFFFF) {
    //get user input
    gameMode = (GameMode)(ToInt(StringInput()));
    
    //Check for validity
    if(gameMode < GAMEMODE_ENTRY_COUNT) {break;}

    //validity failed, so reset the value, show message and reloop
    gameMode = 0xFFFFFFFF;
    cout << "Please enter a valid choice using the number next to your desired game mode." << endl;
    cout << "Press ENTER to try again..." << endl;
    cin.get();
    DisplayGameMode();
  }

  return gameMode;
}

Depending on your compiler you may need to typecast when you assing an int to a GameMode typed variable. Alternatively you could use a bool to break the loop, which would may be a little easier to read.


#4Khatharr

Posted 23 December 2012 - 03:08 AM

That did make it a lot smaller and got rid of the huge switch-case.  That was the next thing. something just seemed weird have that huge switch-case and that also seemed to be repeating it's self a lot.

 

Couple questions:

1: I can change my enum to start at 1 can't I?  Just saying because that is what my menu choices start from.

 

2: Instead of returning some invalid value to have the compiler not complain (I assume it'd be from not all paths returning a value, unless I am wrong?) could it be better to just set the choice to a temporary gameMode variable and then return that temp?  I say that only because I've never seen (though I also haven't looked at a lot of peoples code) a function just return some junk value, even though that part of the code would never get executed (unless I am missing something).  Though now that I think about it, since that function should never return the junk value I guess it really wouldn't matter.

 

Thanks for all the input so far everyone.

 

1) Yes, you can define the value of any enum member like I did to set the zero there for COMPUTER_PLAYER. If you do that in this case though you'll need to remember to check that the value is non-zero when you test for validity. It may be better to just subtract 1 from the input value. If you do decide to change the start value then that last value should probably be renamed to something like GAMEMODE_TERMINATOR, since it no longer represents the count of valid members.

 

2) Some people get edgy about functions that have more than one exit point. In large or complex functions this can definitely be a hazard since someone changing the function later may add something that needs to be done every call but they don't notice that there's a possible return at an earlier point. You could just as easily do this:

 

GameMode Game::GetGameMode() {
  GameMode gameMode = 0xFFFFFFFF;
  while(gameMode == 0xFFFFFFFF) {
    //get user input
    gameMode = (GameMode)(ToInt(StringInput()));
    
    //Check for validity
    if(gameMode < GAMEMODE_ENTRY_COUNT) {break;}

    //validity failed, so reset the value, show message and reloop
    gameMode = 0xFFFFFFFF;
    cout << "Please enter a valid choice using the number next to your desired game mode." << endl;
    cout << "Press ENTER to try again..." << endl;
    cin.get();
    DisplayGameMode();
  }

  return gameMode;
}

Depending on your compiler you may need to typecast when you set an in to a mode. Alternatively you could use a bool to break the loop, which would may be a little easier to read.


PARTNERS