Bad Code question

[kylcrow]

I am writing code for a homework assignment. I have to write a function, and in the function prompt the user to enter either rock, paper, or scissors. I did it successfully declaring a char (choice) globably, but I was told this wasnt a good idea, even though it worked. Why wouldn't this be a good idea?

actually here is my code so far...

#include <iostream>
#include <ctime>
#include <cstdlib>
using namespace std;
char getUserChoice(char);
char choice;
int main()
{
char again;
int computer = 0;
int player = 0;
int tie = 0;
do {
char comp;
srandom(time(0));
int n = random() % 3;
n++;
 
if (n == 1) {
comp = 'R';
} else if (n == 2) {
comp = 'P';
} else {
comp = 'S';
}
 
if (comp == getUserChoice(choice)) {
cout << "It's a tie.\n";
tie += 1;
} else if ((comp == 'R') && (choice == 'S')) {
cout << "Rock smashes scissors. You Lose.\n";
computer += 1;
} else if ((comp == 'P') && (choice == 'R')) {
cout << "Paper covors rock. You Lose.\n";
computer += 1;
} else if ((comp == 'S') && (choice == 'P')) {
cout << "Scissors cut paper. You Lose.\n";
computer += 1;
} else if ((comp == 'R') && (choice == 'P')) {
cout << "Paper covors rock. You Win.\n";
player += 1;
} else if ((comp == 'P') && (choice == 'S')) {
cout << "Scissors cut paper.You Win.\n";
player += 1;
} else if ((comp == 'S') && (choice == 'R')) {
cout << "Rock smashes scissors.You Win.\n";
player += 1;
}
cout << "Computer wins: " << computer << endl << "User wins: " << player << endl << "Ties: "
<< tie << endl;
 
cout << "Play again?(Yes or No): ";
cin >> again;
cin.ignore(1000, '\n');
again = toupper(again);
} while (again == 'Y')
;
return(0);
}
 
char getUserChoice(char)
{
cout << "Please enter Rock, Paper, or Scissors: ";
cin >> choice;
cin.ignore(1000, '\n');
choice = toupper(choice);
return(choice);
}

How would I return the users input using a function without returning a variable? char getUserChoice()

[Infarction]

Your code really needs to be indented for readability purposes.

Anyways, global identifiers are typically considered bad style. They still work, but if you can come up with a solution without globals, then you should do that. And since you return the user's choice anyways, why do you need to keep it in a global variable?

[iamthwee]

> `srandom' undeclared (first use this function)

Depending on what compiler you are using that srandom function may not be recognised.

[kylcrow]

Thats what I was really needing. A way to return an R S or P without returning a variable. Im not expecting someone to do it for me, just point me in the right direction. (Sorry about the indents I am trying to copy and paste from putty and its not extremely simple.

[iamthwee]

Well the problem is your comparison. You cannot use == to compare chars. Wait you're using a single char... oops.

[mariocatch]

i took his code and obviously put the srandom to srand and the random to rand... works fine

[kylcrow]

The code worked fine the way I had it with my compiler, I was just asking how I could change it so that the function wouldn't need to return a variable... would this work?

char getUserChoice ()
{
int again = 0;
do {
char x;
cout << "Please enter Rock, Paper, or Scissors: ";
cin >> x;
cin.ignore(1000, '\n');
x = toupper(x);
if (x == 'R') {
return ('R');
} else if (x == 'P') {
return ('P');
} else if (x == 'S') {
return ('S');
} else {
again = 1;
cout << "Not a good choice. ";
}
} while (again = 1);
}

[iamthwee]

Maybe you need to read a tutorial about functions and the scope of variables within a function? Because you have more or less got it. You just have to rename a few of your variables.

[mariocatch]

you tell us lol... does it work?

[iamthwee]

#include <iostream>
#include <ctime>
#include <cstdlib>

using namespace std;

char getUserChoice ( void );


int main()
{
   char again;
   int computer = 0;
   int player = 0;
   int tie = 0;
   do
   {
      char comp;
      srand ( time ( 0 ) );
      int n = rand() % 3;      
      n++;

      if ( n == 1 )
      {
         comp = 'R';
      }
      else if ( n == 2 )
      {
         comp = 'P';
      }
      else
      {
         comp = 'S';
      }
      char summat = getUserChoice();
      if ( comp ==  summat  )
      {
         cout << "It's a tie.\n";
         tie += 1;
      }
      else if ( ( comp == 'R' ) && ( summat == 'S' ) )
      {
         cout << "Rock smashes scissors. You Lose.\n";
         computer += 1;
      }
      else if ( ( comp == 'P' ) && ( summat == 'R' ) )
      {
         cout << "Paper covors rock. You Lose.\n";
         computer += 1;
      }
      else if ( ( comp == 'S' ) && ( summat == 'P' ) )
      {
         cout << "Scissors cut paper. You Lose.\n";
         computer += 1;
      }
      else if ( ( comp == 'R' ) && ( summat == 'P' ) )
      {
         cout << "Paper covors rock. You Win.\n";
         player += 1;
      }
      else if ( ( comp == 'P' ) && ( summat == 'S' ) )
      {
         cout << "Scissors cut paper.You Win.\n";
         player += 1;
      }
      else if ( ( comp == 'S' ) && ( summat == 'R' ) )
      {
         cout << "Rock smashes scissors.You Win.\n";
         player += 1;
      }
      cout << "Computer wins: " << computer << endl << "User wins: " << player << endl << "Ties: "
      << tie << endl;

      cout << "Play again?(Yes or No): ";
      cin >> again;
      cin.ignore ( 1000, '\n' );
      again = toupper ( again );
   }
   while ( again == 'Y' );
   return ( 0 );
}

char getUserChoice ( void )
{
   char choice;   
   cout << "Please enter Rock, Paper, or Scissors: ";
   cin >> choice;
   cin.ignore ( 1000, '\n' );
   choice = toupper ( choice );
   return ( choice );
}

[kylcrow]

Well that code returns what I need. Either an R, S, or P. But what needs to be displayed is the users choice. Do I have to assign the user's choice to a variable in the main function or is there a way to display what the function returned without displaying the same question again.

example...

User picked: R //what i need

User picked: Please enter r, p, or s:
R // what i get

[Aredhead]

Thats what I was really needing. A way to return an R S or P without returning a variable.

Then considder this small example:

#include <stdio.h>

void get_ch(char*);

  void main(){
    char ch;
    get_ch(&ch);
    printf("%c\n", ch);
}

void get_ch(char* c){
    printf("Please input char: ");
    fflush(stdout);
    while (*c = fgetc(stdin)){
        if(*c == '\n')
            continue;
        if((*c >= 'a') && (*c <='z')){
            *c = (*c & (~32));
            break;
        }
        else
            if((*c >= 'A') && (*c <= 'Z'))
                 break;
        printf("Error: invalid input: %c\n", *c);
        printf("Please input char: ");
        fflush(stdout);
    }
}

[iamthwee]

>But what needs to be displayed is the users choice.

Have a go what do you think?

[kylcrow]

Ok, my first idea would be to try this...

char userChoice;
 
userChoice = getUserChoice();
cout << "User picked: " << userChoice << endl;

But I tried that and I got this warning...

cc1plus: warnings being treated as errors
hw3b.cc: In function 'char getUserChoice()':
hw3b.cc:63: warning: control reaches end of non-void function

[iamthwee]

can you post the entire code please.

[kylcrow]

#include <iostream>
#include <ctime>
#include <cstdlib>
using namespace std;
char getUserChoice();
char getCompChoice();
int main()
{
  char userChoice;
  userChoice = getUserChoice();
  cout << "User picked: " << userChoice << endl;
  cout << "Computer picked: " << getCompChoice() << endl;
  return (0);
}

char getUserChoice ()
{
  int again = 0;
  do {
    char x;
    cout << "Please enter Rock, Paper, or Scissors: ";
    cin >> x;
    cin.ignore(1000, '\n');
    x = toupper(x);
    if (x == 'R') {
      return ('R');
    } else if (x == 'P') {
      return ('P');
    } else if (x == 'S') {
      return ('S');
    } else {
      again = 1;
      cout << "Not a good choice. ";
    }
  } while (again == 1);
}

char getCompChoice()
{
  srandom(time(0));
  int n = random() % 3;
  n++;
  if (n == 1) {
    return ('R');
  } else if (n == 2) {
    return ('P');
  } else {
    return ('S');
  }
}

Here it is.

[iamthwee]

hmm..

#include <iostream>
#include <ctime>
#include <cstdlib>
using namespace std;

char getUserChoice();
char getCompChoice();

int main()
{
   char userChoice;
   char compChoice;   
   userChoice = getUserChoice();
   compChoice = getCompChoice();
   cout << "User picked: " << userChoice << endl;
   cout << "Computer picked: " << compChoice << endl;

   cin.get();
   return ( 0 );
}

char getUserChoice ()
{
   int again = 0;
   do
   {
      char x;
      cout << "Please enter Rock, Paper, or Scissors: ";
      cin >> x;
      cin.ignore ( 1000, '\n' );
      x = toupper ( x );
      if ( x == 'R' )
      {
         return ( 'R' );
      }
      else if ( x == 'P' )
      {
         return ( 'P' );
      }
      else if ( x == 'S' )
      {
         return ( 'S' );
      }
      else
      {
         again = 1;
         cout << "Not a good choice. ";
      }
   }
   while ( again == 1 );
}

char getCompChoice()
{
   srand ( time ( 0 ) );
   int n = rand() % 3;
   n++;
   if ( n == 1 )
   {
      return ( 'R' );
   }
   else if ( n == 2 )
   {
      return ( 'P' );
   }
   else
   {
      return ( 'S' );
   }
}

[kylcrow]

Same error came up. By the way, you have really been a great help. I am understanding better, your not just telling me the answer, that is what I was looking for in a forum. so Thanks.

[iamthwee]

>Same error came up.

Ok this may be a compiler issue, cos it worked here. I'm not entirely sure what it could be although my hunch is this line (and variants thereof) might be the issue:-

if ( x == 'R' )
      {
         return ( 'R' );     
      }

Then again I could be wrong?

char getUserChoice(void); <- you might need a void there also?

>I am understanding better, your not just telling me the answer, that is what I was looking for in a forum. so Thanks.

Yes, tis true, but it also happens to be very time consuming. He he.

[iamthwee]

What compiler are you using?