This is similar to the game War, but with notable differences. For this program, it is the player versus the computer (so each with half of the deck). They draw the top card of their deck and compare. Whoever has the higher value card wins both cards and put both cards into the bottom of their decks (first the card the winner chose then the card the loser chose). Ranks and suits do matter when comparing. Order of rank and suits from highest to lowest:

• Ranks: King, Queen, Jack, 10, 9, 8, 7, 6, 5, 4, 3, 2, Ace


• Suits: Hearts, Diamonds, Clubs, Spades

The winner of the entire match, (or Duel as I call it in the program), is whoever ends up with the entire deck in their possession. Since the games generally get to several hundred moves, the game is automated.

/*

Card game in which the two players each have half of the deck.

Taking the top card of their deck, they compare the cards, seeing who has the

higher ranked card (compare rank then suit). Winner takes the two cards. To win

the game, the player must have the entire deck in his possession.

For this game, it will be the user versus the computer.

Based on the children's game, War

*/



#include <stdio.h>

#include <string.h>

#include <ctype.h>

#include <time.h>

#include <stdlib.h>

#include <unistd.h>



#define FALSE 0

#define TRUE 1



// Define suits, ranks, and respective names

enum Suit { Spades, Clubs, Diamonds, Hearts};

enum Rank { Ace, Two, Three, Four, Five, Six, Seven, Eight, Nine, Ten, Jack, Queen, King};

char * SuitNames = {"Spades", "Clubs", "Diamonds", "Hearts"};

char * RankNames = {"Ace", "Two", "Three", "Four", "Five", "Six", "Seven", "Eight", "Nine", "Ten", "Jack", "Queen", "King"};



// struct for a card

typedef struct CardStructure

{

    enum Suit suit;

    enum Rank rank;

} Card;



// Method prototyping

void playMove(Card * playerHand, Card * compHand);

void dealCards(Card * playerHand, Card * compHand, Card * deck);

int compareCards(Card * playerCard, Card * compCard);

int checkWin(Card * playerHand, Card * compHand);





int main()

{

    Card * playerHand[52];

    Card * compHand[52];

    Card * deck[52];

    char playAgain;



    puts("Welcome to War! You literally duel the computer by comparingn"            // Basic greeting and introduction

           "your top card with the computer's top card. Whoever has the highestn"

           "value card takes the two cards and puts them onto the bottom of theirn"

           "deck. The winner of the game is determined by whoever ends with then"

           "entire deck in their possession.n"

           "From highest to lowest:n"

           "tRanks: A K Q J 10 9 8 7 6 5 4 3 2n"

           "tSuits: Hearts, Diamonds, Clubs, Spadesn"

           "The game will start shortly.n");

    sleep(25);                                   // Give user time to read the greeting



    // Main game loop

    do

    {

        dealCards(playerHand, compHand, deck);   // Give the player and computer half the deck

        int moves = 1;                           // Keeps track of number of moves



        while (1)

        {

            printf("Move: %dn", moves);

            playMove(playerHand, compHand);                 // Player and computer draw their top card and compare, finish turn

            int result = checkWin(playerHand, compHand);

            if (result == 1)

            {

                puts("The player has won the duel!");

                break;

            }

            else if (result == 2)

            {

                puts("The computer has won the duel!");

                break;

            }



            moves++;

        }



        printf("nnWould you like to play again? Enter Y for yes, anything else for no: ");  // Prompt to restart game

        scanf(" %c", &playAgain);

        if (toupper(playAgain) != 'Y')

        {

            for (int index = 0; index < 52; index++)

            {

                free(deck[index]);          // Cards were malloced in dealCards(), time to free them

            }

            break;

        }

    } while (1);

    return 0;

}





void dealCards(Card * playerHand, Card * compHand, Card * deck)

{

    int cardsCreated = 0;

    int turn = 0;         // Keeps track of who is supposed to get the card, player = 0, computer = 1

    Card card;            // Card template

    srand(time(NULL));    // Randomize the seed



    while (cardsCreated < 52)

    {

        int cardFound = FALSE;

        card.rank = rand() % 13;

        card.suit = rand() % 4;



        for (int index = 0; index < cardsCreated; index++)

        {

            Card * deckCard = deck[index];  // Take a card from the deck...



            if (deckCard->rank == card.rank && deckCard->suit == card.suit) // ...and compare it to the newly made card

            {

                cardFound = TRUE;  // Card is a duplicate, exit loop and continue

                break;

            }



        }



        if (cardFound == FALSE)

        {

            if (turn == 0) {

                playerHand[cardsCreated/2] = ( Card *) malloc ( sizeof(Card)); // Malloc the card and give player the card

                playerHand[cardsCreated/2]->suit = card.suit;

                playerHand[cardsCreated/2]->rank = card.rank;

                deck[cardsCreated] = playerHand[cardsCreated/2];               // Add card to deck for comparison purposes

            }

            else if (turn == 1) {

                compHand[(cardsCreated-1)/2] = ( Card *) malloc ( sizeof(Card)); // Malloc the card and give computer the card

                compHand[(cardsCreated-1)/2]->suit = card.suit;

                compHand[(cardsCreated-1)/2]->rank = card.rank;

                deck[cardsCreated] = compHand[(cardsCreated-1)/2];               // Add card to deck for comparison purposes

            }



            turn = (turn == 0) ? 1 : 0;    // Switch turn from 0 -> 1 or 1 -> 0

            cardsCreated++;

        }

    }



    for (int index = 26; index < 52; index++)   // Set the non-existent cards to NULL

    {

        playerHand[index] = NULL;

        compHand[index] = NULL;

    }

}







void playMove(Card * playerHand, Card * compHand)

{

    Card * playerCard = playerHand[0];      // Get top cards and their information

    Card * compCard = compHand[0];

    int pSuit = playerCard->suit;

    int pRank = playerCard->rank;

    int cSuit = compCard->suit;

    int cRank = compCard->rank;

    int result = compareCards(playerCard, compCard);    // If player has the better card, returns 0, otherwise returns 1



    if (!result)

    {

        printf("The player won this turn.n"

               "tPlayer's card: %s of %sn"

               "tComputer's card: %s of %sn", RankNames[pRank], SuitNames[pSuit], RankNames[cRank], SuitNames[cSuit]);

        for (int index = 1; index < 52; index++)

        {

            playerHand[index-1] = playerHand[index];    // Shifts every card forward (subtracts one from their index)

            compHand[index-1] = compHand[index];

        }



        int length = 0;

        for (int index = 0; index < 52; index++)        // Calculate how many cards in the player's hand with the top card discarded

        {

            if (playerHand[index] != NULL)

                length++;

        }

        playerHand[length] = playerCard;                // Place discarded top card to the bottom of the deck

        playerHand[length+1] = compCard;                // Place the won card to the bottom of the deck

    }



    else

    {

        printf("The computer won this turn.n"

               "tPlayer's card: %s of %sn"

               "tComputer's card: %s of %sn", RankNames[pRank], SuitNames[pSuit], RankNames[cRank], SuitNames[cSuit]);

        for (int index = 1; index < 52; index++)

        {

            playerHand[index-1] = playerHand[index];    // Shifts every card forward (subtracts one from their index)

            compHand[index-1] = compHand[index];

        }



        int length = 0;

        for (int index = 0; index < 52; index++)        // Calculate how many cards in the computer's hand with the top card discarded

        {

            if (compHand[index] != NULL)

                length++;

        }

        compHand[length] = compCard;                    // Place discarded top card to the bottom of the deck

        compHand[length+1] = playerCard;                // Place the won card to the bottom of the deck

    }

}





int compareCards(Card * playerCard, Card * compCard)

{

    if (playerCard->rank > compCard->rank)      // Compares ranks

        return 0;

    else if (playerCard->rank < compCard->rank)

        return 1;

    else

    {

        if (playerCard->suit > compCard->suit)  // If ranks match, compare suits

            return 0;

        else

            return 1;

    }

}





int checkWin(Card * playerHand, Card * compHand)

{

    int playerLen = 0;

    int compLen = 0;



    for (int i = 0; i < 52; i++)        // Number of cards is determined by the number of non-NULL cards

    {

        if (playerHand[i] != NULL)

            playerLen++;

        if (compHand[i] != NULL)

            compLen++;

    }

    printf("Player deck size: %dnComputer deck size: %dn"

           "----------------------------------------------n", playerLen, compLen);     // Output current size of player's and computer's deck



    if (playerLen == 52)    // Player has entire deck, player wins

        return 1;

    if (compLen == 52)      // Computer has entire deck, computer wins

        return 2;

    return -1;              // No one has entire deck, continue play

}

My question is that what can I do to improve the program, specifically around the areas of code logic and style (like the C equivalent of Python's PEP8)?

There's no major issue. Beautiful code.
- That's a matter of taste, but you could define TRUE and FALSE in their logical terms:

#define TRUE  (1==1)

#define FALSE (!TRUE)

• 
  moves could be an unsigned
  

dealCards

int turn = 0;

• 
  turn should be simply renamed computerTurn (see below why)



• and since you defined TRUE/FALSE, use it here : int computerTurn = FALSE;.




• also, if (turn == 0) can be explicitly changed to if (!computerTurn)



• and else if (turn == 1) to else if (computerTurn) or even just else.


• 
  turn = (turn == 0) ? 1 : 0; can be rewritten with the logical unary not: computerTurn = !computerTurn;.


• Instead of writing two times almost the same code...

playerHand[cardsCreated/2] = ( Card *) malloc ( sizeof(Card)); // Malloc the card a...

playerHand[cardsCreated/2]->suit = card.suit;

playerHand[cardsCreated/2]->rank = card.rank;

       ...make cards allocation and creation outside of the condition (see my example below).

• Also, you could omit to cast the result of malloc and use the variable name into sizeof instead of using the type.

Instead of your logic, you can try this:

• Make an array of 52 int :

int cards[52];

for (unsigned int i = 0; i < 52; i++) {

    cards[i] = i;

}

• Then, shuffle it (maybe with the Fisher–Yates shuffle).


• Now you just have to alternatively deal card from this array to player and computer (and after, as you do, set non-existent cards from both hands, to null).

for (unsigned int i = 0; i < 52; i++) {

    Card *current = malloc(sizeof(card));

    current->suit = cards[i] % sizeof(SuitNames)/sizeof(*SuitNames);

    current->rank = cards[i] % sizeof(RankNames)/sizeof(*RankNames);



    if (!computerTurn) playerHand[i/2] = current;

    else playerHand[i/2] = current;



    computerTurn = !computerTurn;

}

I see some advantages of this method:

• You can get rid of the variable deck
  


• The generation and dealing in constant in time, where with your method, if you are out of luck, filling both hands can take long time.

playMove

I didn't understand advantage of pSuit, pRank, cSuit and cRank against their arrowed counter-part.

result should be renamed computerWin it's more explicit.

A lot of duplicated code can be moved out of the if...else (before or after), but above all, a major optimization could be to keep a track of how many cards each hand have (playerCount and compCount).

• You don't have anymore to shift left every card in both deck


• You don't have to compute how many card each hand have


• You also have easily access to the current card (playerHand[playerCount-1])

In fact, maybe you can wrap the "hand" and the "count" into a struct:

typedef struct HandStructure

{

    Card *cards[52];

    size_t count;

} Hand;

compareCards

You can simplify branching a lot:

int compareCards(Card * playerCard, Card * compCard)

{

    if (playerCard->rank <= compCard->rank)      

        return (playerCard->rank < compCard->rank) || (playerCard->suit < compCard->suit)

    else return 0;

}

checkWin

You should return 0 instead of -1 if no one won, it allow if(checkWin(...)) in the main.

Dealing with the Hand structure which I talked about, this function became also shorter and simple:

int checkWin(Hand * player, Hand * computer)

{

    assert(player->count + computer->count == 52);



    if (!computer->count)    // Player has entire deck, player wins

        return 1;

    if (!player->count)      // Computer has entire deck, computer wins

        return 2;

    return 0;                // No one has entire deck, continue play

}

edit: Also, you should define both char* at the top as const.

End words

Also, as you can see, I introduced assertion in the last code, paradoxically to the length of this post, i found code pretty well writes. It's time, I think, to adopt stronger concepts and methods. Assertions are one of those things.

I hope not being too straight or rude, English isn't my primary language, so I miss some nuancies.

My question is that what can I do to improve the program, specifically around the areas of code logic and style ... ?

Use stdbool.h

Since C99 @Lundin, C has the _Bool type which has 2 values: 0 and 1. Use that rather than

// #define FALSE 0

// #define TRUE 1

// int cardFound = FALSE;

// cardFound = TRUE;  

// if (cardFound == FALSE)

use

#include <stdbool.h>

bool cardFound = false;

cardFound = true;  

if (!cardFound)

Flush output

stdout is commonly line buffered, but may be unbuffered or fully buffered. To insure out is seen before requesting input, (especially when that output does not end in a 'n') us fflush().

 printf("nnWould you like to play again? Enter Y for yes, anything else for no: ");

 fflush(stdout);

 scanf(" %c", &playAgain);

Format to presentation width

Code should be auto-formated so re-formating to a different preferred max width should be easy. Reviewing code that rolls far off the right of the screen reduced code review efficiency. OP's width is 143+. Suggest something in the 75-100 range.

char * RankNames = {"Ace", "Two", "Three", "Four", "Five", "Six", "Seven", "Eight", "Nine", "Ten", "Jack", "Queen", "King"};



printf("nnWould you like to play again? Enter Y for yes, anything else for no: ");  // Prompt to restart game

scanf(" %c", &playAgain);



           "----------------------------------------------n", playerLen, compLen);     // Output current size of player's and computer's deck 

vs.

char * RankNames = {"Ace", "Two", "Three", "Four", "Five", "Six", "Seven", 

    "Eight", "Nine", "Ten", "Jack", "Queen", "King"};



// Prompt to restart game

printf("nn" // 

   "Would you like to play again? Enter Y for yes, anything else for no: ");

scanf(" %c", &playAgain);



    // Output current size of player's and computer's deck

    "----------------------------------------------n", playerLen, compLen);

Allocate to the object and drop cast

Consider the below. The first obliges a check: Was the correct type used? The 2nd does not need that check. The unnecessary cast is WET. The 2nd is DRY, easier to code right, review and maintain.

// playerHand[cardsCreated/2] = ( Card *) malloc ( sizeof(Card));

playerHand[cardsCreated/2] = malloc (sizeof *playerHand);

Robust code would also detect allocation failures.

if (playerHand[cardsCreated/2] == NULL) {

  // call out of memory handler

}

Reduce rand() calls

2 calls to rand() is twice the time when one would do.

// card.rank = rand() % 13;

// card.suit = rand() % 4;

int r = rand();

card.rank = (r/4) % 13;

card.suit = r%4;

Remove else

Minor style issue: compareCards() looks like it is missing a return at the function end. (it is not though)

Alternate layout:

int compareCards(Card * playerCard, Card * compCard) {

  if (playerCard->rank > compCard->rank) { // Compares ranks

    return 0;

  }

  if (playerCard->rank < compCard->rank) {

    return 1;

  } 

  if (playerCard->suit > compCard->suit) { // As ranks match, compare suits

    return 0;

  }

  return 1;

}

Other simplification possible.

Employ const

When a function parameter points to unchanged data, make it const. This allows for wider code use, some optimizations and conveys better codes intent.

// int checkWin(Card * playerHand, Card * compHand);

int checkWin(const Card * playerHand, const Card * compHand);

Unused non-standard include

The only reason for the non-standard C <unistd.h> seems to be sleep(). Consider removal for greater portability.

Technical undefined behavior (UB)

toupper(ch) is valid for int values in the unsigned charrange and EOF. Else UB when ch < 0.

    scanf(" %c", &playAgain);

    // if (toupper(playAgain) != 'Y')

    if (toupper((unsigned char) playAgain) != 'Y')

Design - missing dealCards() companion

Rather than code in main() the free-ing for (int index = 0; index < 52; index++) free(deck[index]);, Consider a companion function to un-deal.