using qsort to sort an array

Question

This program is supposed to sort cards, group them and into 5 groups called hands, aparently the code does exactly that, however for some reason am not so sure of, I cannot determine the winer. The winner is supposed to be the value shown as highest among the pairs.eg. if hand one has king as highest pair, and hand 3 has 7 as highest pair, then hand one should be declared a winner.

#include<stdio.h>
#include<stdlib.h>
#include<time.h>
#include <windows.h>


typedef unsigned char card;
typedef unsigned char pairs;

static char *suits[] = {"Hearts","Diamonds","Clubs","Spades"};
static char *values[]= {"Ace","Two","Three","Four","Five","Six",\
                    "Seven","Eight","Nine","Ten","Jack",\
                    "Queen","King"};
static char *colour[]= {"Black","Red"};

void printcard(card c); /* Displays the value of a card*/
void printdeck(card deck[52]); /* prints an entire deck of cards*/
void filldeck(card deck[52]); /* Populates a deck of cards */
void shuffle(card deck[52]); /* Randomizes the order of cards */
int compareface(const void* c1,const void *c2); 

 pairs findpairs(card *hand); /* finds any pairs in a hand */
 int findwinner(card *hand);


 int main()
 {

 card deck[52],*deckp,*temp3,max;
 card hands[5][5],handssorted[5][5];
 pairs numpairs[5],highest;
 int hand,cd,winner=0,temp2;
 char loop;

 { 
 printf("\n(__)(__)(__)(__) CARD GAME(__)(__)(__)(__)(__)\n\n");
 printf("\n(__)(__)(__)(__)(__)(__)(__)(__)(__)(__)(__)(__)\n\n");
 printf("\n\t ====== Before Shuffling =====\n\n");
 srand(time(NULL));  /* seed the random number generator */

  filldeck(deck);//printdeck(deck);
  printdeck(deck);
  printf("\n\t ========== Shuffled Cards ========\n\n");
  shuffle(deck);//printdeck(deck);
  printdeck(deck);

 int i=0;
 for(cd=0;cd<5;cd++)
 {
    for(hand=0;hand<5;hand++){
        hands[hand][cd]=deck[i];
        i++;
    }
  }

 printf("\n\t ============= Hands ===========\n\n");
for(hand=0;hand<5;hand++)
 { 
    qsort(hands[hand],5,sizeof(card),compareface);;

    int kok=0;

    printf("\tHand %i:\n",hand+1);

    for( cd=0;cd<5;cd++)
    {            
        printcard(hands[hand][cd]);
    }

    numpairs[hand]=findpairs(hands[hand]);
    printf("\n\tNumber of pairs:%i\n\n",numpairs[hand]);
    temp2=0;
    if(numpairs!=0)
        temp2=findwinner(hands[cd]);
 }
 pairs findpairs(card *hand)
{     int cd=0,dd=0;

  card temp;
  pairs numpairs=0;

  for (cd=0;cd<4;cd++)
  {
  if (values[(hand[cd]&0x3c)>>2] == values[(hand[cd+1]&0x3c)>>2])
  {
      dd++;
      temp=hand[cd];
  }

  switch (dd)
  {
         case 0:numpairs=0;
         break;
         case 1:numpairs=1;
         break;
         case 2:
              if ((values[(hand[1]&0x3c)>>2] !=values[(hand[3]&0x3c)>>2])&&
               (values [(hand[0]&0x3c)>>2]\
                   !=values[(hand[2]&0x3c)>>2])&&(values[(hand[2]&0x3c)>>2] !=values
                [(hand[4]&0x3c)>>2]))numpairs=2;

              else 
                   numpairs=1;   
                   break;
         case 3:numpairs=2;
         break;
         case 4:numpairs=2;
         break;
         }}
         printf("\tHighest pair is:");
         if (numpairs!=0)
         printf("\t%s",values[(temp&0x3c)>>2]);         
  return numpairs;
 }

int findwinner(card *hand){     
int cd=0,temp2=0,winner=0;
for (cd=0;cd<4;cd++)
  {
   if (values[(hand[cd]&0x3c)>>2] == values[(hand[cd+1]&0x3c)>>2])
  {
    // temp2=cd;
    winner=cd;
  }}

  return winner;
 }


void filldeck(card deck[52])
 {
 card suit,value;
 int i=0;

 for(suit=0;suit<4;suit++)
 {
     for(value=0;value<13;value++)
     {
         deck[i]=suit|(value<<2);                     
         if(suit<2)

           deck[i]|=0x40;  /*card is red, so do red stuff */                         
         i++;
     }
 }

return;
 }
void printdeck(card deck[52])
{
  int i;
  for(i=0;i<52;i++)
  printcard(deck[i]);
   return;
}

void printcard(card c)
{
  printf("\t%s\t of  %-8s  is  %s\n",values[(c&0x3c)>>2],suits[c&0x3],
  colour[(c&0x40)  >>6]);
  return;
}

void shuffle(card deck[52])
{
  int i,rnd;
  card c;
  for(i=0;i<52;i++)
 {
  rnd=rand() * 52.0 / RAND_MAX; 
  c=deck[rnd];
  deck[rnd]=deck[i];
  deck[i]=c;  
 } 
  return;
 }


int compareface(const void* c1, const void *c2)
  {

    card cd1,cd2;
    cd1=*((card*) c1);
    cd2=*((card*) c2);

    cd1= (cd1&0x3c)>>2;
    cd2= (cd2&0x3c)>>2;

    if(cd1>cd2)
        return 1;

    if(cd1==cd2)
        return 0;

    return -1;
}

Answer 1

One probable problem is at the end of the 'compare' loop:

if(numpairs!=0)
    temp2=findwinner(hands[cd]);

Since 'cd' is out of range at the time, you get odd results. You would have to explain more for us to be sure about what is right.

You should unit test your sort code. The shift is not really necessary; the bit masking ensures that the values are monotonically increasing ignoring suit, which is presumably what is being shifted out.

---

I've taken a look at the complete code (which is missing a pair of close braces before the function findpairs() is defined). It doesn't need the <windows.h> header to compile, which is good. The core code - that which shuffles the cards etc - seems to be solid enough. There are lots of nit-picky changes I'd make, but it is OK.

One subtle detail: the card comparison routine doesn't sort the array '2H 2C 2S 2D 3S' so that the deuces are in any specific suit order. This might matter later; the person with '2C 2S' might win over the person with '2D 2H' because 2S is ranked higher than any of the other deuces. And if that's not the case, you still need to nail down what the rules are. You also need to consider how many pairs there are in the hand '2H 2C 2S 2D 3S' - 6 would be a reasonable answer.

The findpairs() code has an interesting piece in the middle; I think you should be using a double loop, with the inner loop finding the end of the range of equal value cards.

The lines I originally highlighted are where your problems start - I don't think you've properly formulated how you determine the winner because you aren't keeping the necessary information. The condition 'numpairs!=0' is always true because numpairs is an array, and the pointer is never the null pointer.

Also, when the findwinner() function is called, cd is 6, and therefore the behaviour is undefined. Since you immediately throw the returned value away, some of it doesn't matter.

Clearly, you need to work hard on:

• How to handle triples and quadruples
• How to determine winners

But your original question about using qsort() can be answered "you are using it OK".

I note, in passing, that there are places where you compare card values for equality with expressions like:

if (values[(hands[hand][cd]&0x3C)>>2] == values[(hands[hand][cd]&0x3C)>>2])

There is no need to reference the values array in this. It works OK, but it is unnecessary.

---

Revised code - no determination of winner, but slightly neater in other respects. Can be compiled with '-DVERBOSE_NAMES' to have cards printed out with long names. Otherwise, it uses short names: 'AC' for ace of clubs, '2H' for deuce of hearts, 'TD' for ten of diamonds, and 'KS' for king of spades. It requires a C99 (or C++) compiler because of the way it declares variables in loops and uses inline functions, etc. (Tested: GCC 4.5.1 and G++ 4.5.1 on MacOS X 10.6.4.)

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

enum { NUM_HANDS      =  5 };
enum { CARDS_PER_HAND =  7 };
enum { CARDS_PER_DECK = 52 };

typedef unsigned char card;
typedef unsigned char pairs;

static const char *values[]=
{
    "Ace", "Two", "Three", "Four", "Five", "Six",
    "Seven", "Eight", "Nine", "Ten", "Jack", "Queen", "King"
};
#ifdef VERBOSE_NAMES
static const char *suits[] = { "Hearts", "Diamonds", "Clubs", "Spades" };
static const char *colour[]= { "Black", "Red" };
#endif /* VERBOSE_NAMES */

static void printcard(card c); /* Displays the value of a card*/
static void printdeck(card deck[CARDS_PER_DECK]); /* prints an entire deck of cards*/
static void filldeck(card deck[CARDS_PER_DECK]); /* Populates a deck of cards */
static void shuffle(card deck[CARDS_PER_DECK]); /* Randomizes the order of cards */
static int  compareface(const void* c1, const void *c2);
static pairs findpairs(card *hand); /* finds any pairs in a hand */

static inline int card_value(card c)  { return((c & 0x3C) >> 2); }
static inline int card_suit(card c)   { return((c & 0x03)); }
static inline int card_colour(card c) { return((c & 0x40) ? 1 : 0); }

static void deal(card deck[CARDS_PER_DECK], card hands[NUM_HANDS][CARDS_PER_HAND])
{
    int i = 0;
    for (int cd = 0; cd < CARDS_PER_HAND; cd++)
    {
        for (int hand = 0; hand < NUM_HANDS; hand++)
            hands[hand][cd] = deck[i++];
    }
}

static void printhand(int num, card hand[CARDS_PER_HAND])
{
#ifdef VERBOSE_NAMES
    printf("\tHand %i:\n", num);
#else
    printf("\tHand %i:", num);
#endif /* VERBOSE_NAMES */
    for (int cd = 0; cd < CARDS_PER_HAND; cd++)
    {
        printcard(hand[cd]);
    }
}

static void printhands(card hands[NUM_HANDS][CARDS_PER_HAND])
{
    int i;
    for (i = 0; i < NUM_HANDS; i++)
    {
        printhand(i+1, hands[i]);
        putchar('\n');
    }
}

int main()
{
    card deck[CARDS_PER_DECK];
    card hands[NUM_HANDS][CARDS_PER_HAND];
    pairs numpairs[NUM_HANDS];

    printf("(__)(__)(__)(__) CARD  GAME (__)(__)(__)(__)(__)\n");
    printf("(__)(__)(__)(__)(__)(__)(__)(__)(__)(__)(__)(__)\n\n");
    printf("\t====== Before Shuffling =====\n\n");
    srand(time(NULL));  /* seed the random number generator */

    filldeck(deck);
    printdeck(deck);
    printf("\t========== Shuffled Cards ========\n\n");
    shuffle(deck);
    printdeck(deck);
    deal(deck, hands);
    printf("\t============= Hands ===========\n\n");
    printhands(hands);
    putchar('\n');

    printf("\t============= Analysis ===========\n\n");
    for (int hand = 0; hand < NUM_HANDS; hand++)
    {
        qsort(hands[hand], CARDS_PER_HAND, sizeof(card), compareface);;
        printhand(hand+1, hands[hand]);
        numpairs[hand] = findpairs(hands[hand]);
    }

    return 0;
}

static pairs findpairs(card *hand)
{
    card pair = 0;
    pairs numpairs = 0;

    for (int cd1 = 0; cd1 < CARDS_PER_HAND; cd1++)
    {
        for (int cd2 = cd1 + 1; cd2 < CARDS_PER_HAND; cd2++)
        {
            if (card_value(hand[cd1]) == card_value(hand[cd2]))
            {
                numpairs++;
                pair = hand[cd1];
            }
        }
    }

    if (numpairs > 0)
        printf("    %d pairs - highest pair is: %s\n", numpairs,
              values[card_value(pair)]);
    else
        printf("    0 pairs\n");
    return numpairs;
}

void filldeck(card deck[CARDS_PER_DECK])
{
    int i = 0;
    for (int suit = 0; suit < 4; suit++)
    {
        for (int value = 0; value < 13; value++)
        {
            deck[i] = suit | (value<<2);
            if (suit < 2)
                deck[i] |= 0x40;  /*card is red, so do red stuff */
            i++;
        }
    }
}

void printdeck(card deck[CARDS_PER_DECK])
{
    for (int i = 0; i < CARDS_PER_DECK; i++)
    {
        printcard(deck[i]);
        if ((i % 13) == 12)
            putchar('\n');
    }
    putchar('\n');
}

#ifndef VERBOSE_NAMES
static char abbr_card_value(card c)
{
    static const char abbr_values[] = "A23456789TJQK";
    return abbr_values[card_value(c)];
}
static char abbr_card_suit(card c)
{
    static const char abbr_suits[] = "CDHS";
    return abbr_suits[card_suit(c)];
}
#endif /* VERBOSE_NAMES */

void printcard(card c)
{
#ifdef VERBOSE_NAMES
    printf("\t%s\t of  %-8s  is  %s\n", values[card_value(c)], suits[card_suit(c)],
           colour[card_colour(c)]);
#else
    printf("  %c%c", abbr_card_value(c), abbr_card_suit(c));
#endif /* VERBOSE_NAMES */
}

void shuffle(card deck[CARDS_PER_DECK])
{
    for (int i = 0; i < CARDS_PER_DECK; i++)
    {
        int rnd = rand() * 52.0 / RAND_MAX;
        card c = deck[rnd];
        deck[rnd] = deck[i];
        deck[i] = c;
    }
}

int compareface(const void* c1, const void *c2)
{
    card cd1 = *((card*) c1);
    card cd2 = *((card*) c2);

    cd1 = card_value(cd1);
    cd2 = card_value(cd2);

    if (cd1 > cd2)
        return +1;
    else if (cd1 < cd2)
        return -1;
    else
        return  0;
}

Answer 2

1. If you really only need this to get the maximum pair, this is really the wrong strategy: too complicated and unnecessarily slow. Just use a for loop to run through your deck and pick the largest pair that you have seen so far.
2. Your comparison code for the cards shows that you have not modularized your program enough. If you insist to code your cards in an integer as it seems, don't use shifts and alike to access the different parts, but bitfields.