Expert: Ralph McArdell - 12/14/2008

Question
Hi, I'm trying to make a program if a user inputs male,m,f, MALE, female, maLE, mAle, feMAle and all sorts of males and females but if it inputs "make me" then it gives an error message and loops again to ask the gender again. This is what I've done:

#include <cstdlib>
#include <iostream>
#include <string>
#include <iomanip>

using namespace std;

int main(int argc, char *argv[])
{
  string gender ;

  // accept string from user
  cin >> gender ;
  
  bool islowercase(string str)
  // first convert all chars to lower case
  for(int i=0 ; i < str.length() ; ++i )
      str[i] = tolower( str[i] ) ;

  bool ok = true ;

  // an empty string is not ok
  if( str.empty() ) ok = false ;

  else if( gender.size() == 1U )
  {
      // a single char was entered, must be either 'f' or 'm'
      if( ( str[0] != 'm' ) && ( str[0] != 'f' ) ) ok = false ;
  }

  // more than one char entered, must be either "male" or "female"
  else if( ( gender != "male" ) && ( gender != "female" ) ) ok = false ;

  if( !ok )
  {
  cout << "ERROR! gender is incorrect\n" ;
  }
}
   system("PAUSE");
   return EXIT_SUCCESS;
}

Where have I gone wrong? This is not a homework question

Answer
I have no idea what you have done wrong in the code you are actually executing because the code you posted seems not to be the code you are having problems with. I say this because the code you posted is in not in a compilable state.

Compiling your code as posted with Microsoft Visual C++ 2008 I received the following errors:

   error C2143: syntax error : missing ';' before 'for'
   error C2065: 'str' : undeclared identifier
   error C2228: left of '.length' must have class/struct/union
       type is ''unknown-type''
   error C2065: 'str' : undeclared identifier
   error C2065: 'str' : undeclared identifier
   error C2065: 'str' : undeclared identifier
   error C2228: left of '.empty' must have class/struct/union
       type is ''unknown-type''
   error C2065: 'str' : undeclared identifier
   error C2065: 'str' : undeclared identifier
   error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
   error C2365: 'system' : redefinition; previous definition was 'function'
       C:\Applications_x86\Microsoft Visual Studio 9.0\VC\include\stdlib.h(521) : see declaration of 'system'
   error C2440: 'initializing' : cannot convert from 'const char [6]' to 'int'
       There is no context in which this conversion is possible
   error C2059: syntax error : 'return'
   error C2059: syntax error : '}'
   error C2143: syntax error : missing ';' before '}'
   error C2059: syntax error : '}'
   error C2143: syntax error : missing ';' before '{'
   error C2447: '{' : missing function header (old-style formal list?)

If you are asking me to look at problems with code then please, please, please:

-   Reduce the problem to as small a piece of code as possible.
   I do _not_ have time to wade through pages and pages of code.

-   Make sure the code is tidy and laid out in a decent fashion.
   E.g. make sure your code is indented in a consistent and reasonable fashion.
   I do not have time to work out the meaning of badly formatted code.

-   Make sure the code posted is in fact the same as the code you are building and executing.
   No point in asking about code you have not posted.

-   If you have errors from a compiler then please show the errors and indicate which lines in your
   posted code the errors refer to as line numbers referred to in compiler errors (obviously) become
   meaningless when code is posted in your question. I do not have time to try and spot which part of
   posted code a posted compiler error refers to. I should point out (as people also do this) that
   just stating you had errors or the program broke or did not work as expected and not saying what
   the errors are or how the program mis-functioned is no help either.

Also mentioning the compiler (including version) e.g. Visual C++ 2008 or GNU g++ 4.2.4, and possibly the operating system (e.g. Windows XP or Ubuntu Linux 8.10) can be of use, as well as any non-standard libraries you are using. I guess from your use of "pause" with system that you are using a Microsoft operating system (this command is not available on say UNIX and Linux systems using their command shells). In this case I think the main problem is not the compiler/operating system so much as the code being a) the wrong code and b) has errors that would be picked up by any C++ compiler.

Here are a few things about your _posted_ code I noticed in passing:

-   You use the form of main with command line argument parameters but are not using them
   so maybe just using the form with no arguments would be better:

       int main()

-   What is this line for, and why does the statement have no terminating semi-colon?

      bool islowercase(string str)

-   You declare, define and start using a single string called gender then start using a (string?) object called str - eh?

-   You go to a lot of trouble using negative logic to see if what you have does not match what you expect.
   Why not make life easier and set ok to false to start with and only set it to true if it matches one
   of the required inputs?

-   You can use single character string literals as well as single character literals (e.g. "f" as well as 'f')

-   Your check for str being one character long seems a little redundant and serves only to make the code
   even more complex. As does you check for the string being empty I think.

-   You can assign the results of Boolean expressions directly to a bool type object (or use them to
   initialise a bool object). Combining this with the previous 4 points leads us to being able to
   write the following to set the value of ok - which I think you will agree is both much shorter and
   much clearer than the code you had (and uses the correct, defined, object - gender - rather than str):

       bool ok( gender=="female" || gender=="male" || gender=="f" || gender=="m" );

-   You say your code loops around on bad inputs so the user can enter a correct value.
   The code you presented has only one loop used when converting the input string to lower case.

-   However you may have left a terminating closing brace ( } ) where your (non-existent)
   loop might have ended (thus causing main to actually end earlier than expected).

As I said these are just those things I noticed from a quick look at your _posted_ code. I think if you apply those that remain relevant to the code you are actually using then you might be able to correct your problem - and maybe simplify and clarify the code to boot. However as I have not seen the code you are actually using I cannot say for sure where you have gone wrong. If taking on board the relevant points made here and changing your code to suit does not fix the problem then please post a followup question - remembering to post the actual code in as short and well formatted state as possible.