Inputing characters into arrays in C- follow up

Question

This is a follow up question to a previous question I asked about calculating a straight hand..not exactly the same...This is the method to read in cards- it works- but is there a better way- to do this using console input in C ...

void read_cards(int num_in_rank[], int num_in_suit[])
{
  bool card_exists[NUM_RANKS][NUM_SUITS];
  char ch, rank_ch, suit_ch;
  int rank, suit;
  bool bad_card;
  int cards_read = 0;

  for (rank = 0; rank < NUM_RANKS; rank++) {
    num_in_rank[rank] = 0;
    for (suit = 0; suit < NUM_SUITS; suit++)
      card_exists[rank][suit] = false;
  }

  for (suit = 0; suit < NUM_SUITS; suit++)
    num_in_suit[suit] = 0;

  while (cards_read < NUM_CARDS) {
    bad_card = false;

    printf("Enter a card: ");

    rank_ch = getchar();
    switch (rank_ch) {
      case '0':           exit(EXIT_SUCCESS);
      case '2':           rank = 0; break;
      case '3':           rank = 1; break;
      case '4':           rank = 2; break;
      case '5':           rank = 3; break;
      case '6':           rank = 4; break;
      case '7':           rank = 5; break;
      case '8':           rank = 6; break;
      case '9':           rank = 7; break;
      case 't': case 'T': rank = 8; break;
      case 'j': case 'J': rank = 9; break;
      case 'q': case 'Q': rank = 10; break;
      case 'k': case 'K': rank = 11; break;
      case 'a': case 'A': rank = 12; break;
      default:            bad_card = true;
    }

    suit_ch = getchar();
    switch (suit_ch) {
      case 'c': case 'C': suit = 0; break;
      case 'd': case 'D': suit = 1; break;
      case 'h': case 'H': suit = 2; break;
      case 's': case 'S': suit = 3; break;
      default:            bad_card = true;
    }

    while ((ch = getchar()) != '\n')
      if (ch != ' ') bad_card = true;

    if (bad_card)
      printf("Bad card; ignored.\n");
    else if (card_exists[rank][suit])
      printf("Duplicate card; ignored.\n");
    else {
      num_in_rank[rank]++;
      num_in_suit[suit]++;
      card_exists[rank][suit] = true;
      cards_read++;
    }
  }
}

I know the case statement can have a few optimizations-i.e. using toupper; better representation the card values, etc..Suggestions- please. The code is reflective of C99...

Answer 1

The code looks fine to me. It's actually quite well-done and readable in my opinion.

It seems like you're asking about optimizations - don't worry about optimizing until you're sure that you need it, otherwise it just makes your code less legible and more prone to bugs. In particular, this function is going to spend well over 99.99% of the time it is running just waiting for the user to type, so there is no need to optimize it.

I don't think it's necessarily bad to use fall-through to capture both upper and lowercase input in a switch statement, especially with as few cases as you have. It's just a little more typing.

If there's actually an issue with this code and it's not compiling or not doing what it's supposed to, please describe the problem.

Edit: The only thing you might want to change is to get rid of the explicit cases for ranks 0-9, and instead combine them all into one fall through case with the body:

rank = rank_ch - '0';

That's a relatively common C idiom for converting a number stored as a character to an integer, so it should be legible to other programmers who read it.

Answer 2

Depends on what you're using it for and how you want to input data. If you want to input one card at a time, you can stick with your method (or methods, if you want to make it easier to read) or use another way like gets. If you're looking to read in hands, you can have the user input cards separated by spaces or commas. Then, just read through the string and validate the input.

Answer 3

Rather than using getchar(), I would consider using scanf("%s",str) -- http://www.cplusplus.com/reference/clibrary/cstdio/scanf. You could then store the results in a character array, and loop through the array and compare the characters to their corrosponding hex or decimal values on the ascii chart http://www.s4a.us/support/images/ascii_chart.gif.

int rank = 0;
int suite = 'c';
char[3] buff; // be careful of overflow
printf ("Enter the rank and suite: ");
scanf ("%s",buff); 

for(int i=0; i<2; i++)
{
    // compare elements of buff to those in the ascii chart
}

Answer 4

The code looks OK to me, so I have only minor suggestions:

• Prompt the user for the suit after they've entered the rank, just to make it really obvious what they're suppose to be doing.

• Following from that, check for an illegal rank immediately, and tell the user then, rather than waiting for them to enter the suit.

• Given that you're only expecting 2 characters from the user, why wait for \n or any chars? Just continue once 2 chars have been entered.

• Purely to aid readability, use an enum for the suits, rather than numbers. If this is for poker (and indeed, many other card games, though not bridge), the order of the suits is irrevelevant anyway.

• I agree with other comments about conversion from chars to enums (rank = (rank_ch - '0') - 2). I wouldn't bother with toupper in such a limited case, though.