Cant get my program to work properly, same numbers when dealing cards

Question

So I'm getting pretty far in my program thanks to the help I've gotten from here.

When I'm calling my function to print out random cards, but I get the same card every time.

So to sum it up, player 1 gets 5 of clubs, the next card is still 5 of clubs.. The dealer gets 5 of clubs too...

I used this in a loop to deal out 2 cards, but I still got 5 of clubs and 5 of clubs, so it's always the same cards (not just 5 of clubs, 9 of hearts etc). Its the last bit of code in the function int player1()

Here's how my code looks:

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

#define DIAMONDS 0
#define CLUBS 1
#define HEARTS 2
#define SPADES 3
#define COLOR_SIZE 13
#define NR_OF_SUITS 4
#define DECK_SIZE 52
#define JACK 11
#define QUEEN 12
#define KING 13
#define ACE 1

struct Card
{
    int suit;
    int value;
    int *sum;
};

int player2(struct Card* deck);
int player1(struct Card* deck);
int printCards(struct Card *cardDeck);
void swapCards(struct Card *cardA, struct Card *cardB);
void shuffleCards(struct Card *cardDeck);

//==========================================================================
int main()
{
    srand((unsigned)time(NULL));
    //struct Card deck[DECK_SIZE];      //Statiskt allokerad array
    struct Card * deck; //Dynamiskt allokerad array
    int index;
    int suit_index;

    deck = (struct Card *)malloc(sizeof(struct Card) * DECK_SIZE);
    for (suit_index = 0; suit_index < NR_OF_SUITS; suit_index++)    /* Initiera kortleken */
        for (index = 0; index < COLOR_SIZE; index++)
        {
            deck[suit_index*COLOR_SIZE + index].suit = suit_index;
            deck[suit_index*COLOR_SIZE + index].value = index;
        }
    shuffleCards(deck);
    player1(deck);
    system("pause");
    return 0;
}
//==========================================================================

int printCards(struct Card *cardDeck)
{
    int sum = 0;
    for (int i = 0; i < 1; i++)
    {
        switch (cardDeck[i].value + 1)
        {
        case ACE: printf("Ace ");
            cardDeck[i].value = 11;
            break;
        case JACK: printf("Jack ");
            cardDeck[i].value = 10;
            break;
        case QUEEN: printf("Queen");
            cardDeck[i].value = 10;
            break;
        case KING: printf("King ");
            cardDeck[i].value = 10;
            break;
        default: printf("%d ", cardDeck[i].value);
            break;
        }

        printf("of ");
        switch (cardDeck[i].suit)
        {
        case DIAMONDS: printf("Diamonds ");
            break;
        case HEARTS: printf("Hearts ");
            break;
        case CLUBS: printf("Clubs ");
            break;
        case SPADES: printf("Spades ");
            break;
        default: printf("Something went wrong!! ");
            break;
        }
        printf("\n");
        sum += cardDeck[i].value;
    }
    return sum;
}
//----------------------------------------------------------------------

void swapCards(struct Card * cardA, struct Card *cardB)
{
    struct Card temp;
    temp = *cardA;
    *cardA = *cardB;
    *cardB = temp;
}
//----------------------------------------------------------------------

void shuffleCards(struct Card *cardDeck)
{

    for (int i = 0; i < DECK_SIZE; i++)
        swapCards(&cardDeck[i], &cardDeck[rand() % 52]);
}
//----------------------------------------------------------------------

int player1(struct Card* deck) {
    int i;
    int val = 0;
    int val2 = 0;
    int sum1, sum2, sum3, first;
    char name[20], name2[20];
    //Player 1, gets 2 cards
    gets(name);
    printf("Welcome %s\n", name);
    for (i = 0; i < 2; i++) {
        sum1 = printCards(deck);
        val += sum1;
    }
    printf("%s, your hand is %d\n\n",name, val);

    //Player2, only 1 card so far
    gets(name2);
    printf("Welcome %s\n", name2);
    sum2 = printCards(deck);
    printf("%s, your hand has a total of %d\n\n", name2, sum2);

    //Dealers turn, looping til value is greater or equal to 17
    printf("Dealers turn\nDealers cards are:\n");
    sum3 = printCards(deck);
    val2 += sum3;
    printf("%d Dealer has a total of \n", val2);
    while (val2 < 17) {
        printf("Dealer gets a new card\n");
        sum3 = printCards(deck);
        val2 += sum3;
        printf("Dealers card is %d, dealers value is now %d\n", sum3, val2);
    }
    printf("Dealer has a total of %d\n", val2);
}

Question:

How do I get different cards every time I call my function?

Note:

I've tried using different variables, loops and such but it still wont work. I really need a solution here, because I've tried like everything and I think this is the last problem I have. I know my code isn't the prettiest, but I'll fix that later.

Answer 1

The commenters have already pointed out your mistake: You shuffle your deck, but you always look at the same card without discarding it from the deck. The deck gets smaller when you draw cards, so you need a means to keep track of how many cards are still in the deck or how many cards have already been drawn.

Your code has some design "smells". For example, you keep a pointer to the sum with the card struct, but a sum should be kept for each player, not for a card. A card has a score, of course, but that can be determined in a separate function.

It is good that you organise your code in various functions, but sometimes you make your functions do too much. printCards, for example, not only prints a card, it also draws a card from the deck and keeps a tally of the current score. Drawing a card from the deck, printing a card and calculating the score should be three separate functions. Eventually, you'll want to print cards without drawing them from the deck or get the score without printing it.

I recommend to use a deck structure tat hold up to 52 cards and from which cards can be drawn. Here's an example program that deals only with the card dealing, not with the game of Black Jack. (That's probably the fun part and left to you.)

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

enum {DIAMONDS, CLUBS, HEARTS, SPADES, MAX_SUIT};
enum {ACE, TWO, THREE, FOUR, FIVE, SIX, SEVEN, EIGHT, NINE,
    TEN, JACK, QUEEN, KING, MAX_RANK};

#define MAX_CARD (MAX_RANK * MAX_SUIT)

struct Card {
    int suit;
    int rank;
};

struct Deck {
    struct Card card[MAX_CARD];
    size_t ncard;
};

/*
 *      Initialise deck
 */
void deck_init(struct Deck *deck)
{
    int suit, rank, k = 0;

    for (suit = 0; suit < MAX_SUIT; suit++) {
        for (rank = 0; rank < MAX_RANK; rank++) {
            deck->card[k].suit = suit; 
            deck->card[k].rank = rank;
            k++;
        }
    }

    deck->ncard = k;
}

/*
 *      Get a random card from the deck. It is illegal to call this
 *      when the deck has been exhausted.
 */
struct Card deck_deal(struct Deck *deck)
{
    if (deck->ncard <= 0) {
        fprintf(stderr, "Deck underflow\n");
        exit(1);
    }

    int pick = deck->ncard * (rand() / (1.0 + RAND_MAX));
    struct Card deal = deck->card[pick];

    deck->card[pick] = deck->card[--deck->ncard];

    return deal;
}

/*
 *      Print a card's name to the char buffer str of size len.
 *      Return the buffer, so it can be used in, e.g., printf.
 */
char *card_str(char *str, size_t len, struct Card c)
{
    static const char *sname[] = {
        "Diamonds", "Clubs", "Hearts", "Spades"
    };
    static const char *rname[] = {
        "Ace", "Two", "Three", "Four", "Five", "Six", "Seven",
        "Eight", "Nine", "Ten", "Jack", "Queen", "King"
    };

    snprintf(str, len, "%s of %s", rname[c.rank], sname[c.suit]);
    return str;
}

/*
 *      Determine the card's score.
 */
int card_score(struct Card c)
{
    if (c.rank == ACE) return 11;
    if (c.rank > TEN) return 10;
    return c.rank + 1;
}

/*
 *      Simple example code
 */
int main()
{
    struct Deck deck;
    int score = 0;
    int ncard = 0;

    srand(time(NULL));

    deck_init(&deck);

    while (deck.ncard && score < 17) {
        struct Card c = deck_deal(&deck);
        char buf[32];

        puts(card_str(buf, sizeof(buf), c));
        score += card_score(c);
        ncard++;
    }
    printf("%d cards drawn with a score of %d.\n", ncard, score);

    return 0;
}

Things to note:

• The deck struct keeps the current number of cards still in the deck alongside the cards proper. When it is initialised, the deck is put into "factory order". Only when picking a card, a random decision is made. (You could as well shuffle the deck on initialisation and pick the topmost card if you wanted to, of course.)

• Printing, drawing and evaluating a card are three separate functions. Drawing a card is an action on the deck that yields a card. Printing and evaluating operate on a single card.

• Printing is achieved by printing to a char buffer here. This is somewhat ugly, but C's way to create strings. It has the advantage that you can use the function in a printf directly, but you must provide a char buffer.

• When you have many constants with consecutive values, you can use an enum, which enumerates these constants for you. Enumerated values start at 0, so that you can put an extra value, such as MAX_SUIT and MAX_RANK here, at the end to give you the number of enumerated items.

• The code for picking a card, printing it and evaluating it is a bit longer than yours, because these things have been separated. You can always write a function do do these three things, if you like, without sacrificing the nice modularity. (I suggest that you create a player struct for the player and the dealer, so that the pick-and-evaluate function can easily keep track of the scores.)

Answer 2

Few general remarks,

"I know my code isn't the prettiest, but I'll fix that later."

is part of your problem, your code is growing and you need to start considering separating it into different files, here is an excellent explanation on the topic of file organization.

Additionally, add some comments describing the different, not so obvious and easy to understand, parts of your code like loops, functions, etc. That will help you and any other reader understand the functionality of your code. for example if a function is doing to many things, should you consider breaking it into smaller functions, each making a single task and is this going to make your job easier in terms of finding an error.

Read more about classes and understand whether is a good idea to integrate all of the functions that act on struct Card into one single class.

P.S. Be patient do not expect to solve a problem for a half an hour, read and research more before asking a question, especially when the question is not technical.

Answer 3

Actually after i looked very carefully through the code, i could not find any place in which dealer, player1 and player2 get their own cards from the deck.

All you do in int player1(struct Card* deck) is continiosly use int printCards(struct Card *cardDeck), which ALWAYS prints a single card from the deck. Actually it always prints cardDeck[0], which by that time will never change. a big BUT is that printing cardDeck is not actually assigning cards to different players, it's just that - printing. Maybe you actually should create some struct, which will hold data about a player. Something like

struct Player
{
  char* name; // name of player
  struct listOfcards* list; // a pointer to a list of cards the player has
} 

Where listOfcards is something like this

struct listOfcards
{
  struct card* currCard // pointer to a current card
  struct card* nextCard; // pointer  to next card in list(NULL if no next card)
} 

Be carefull though, this exaple is very simple, you will have to iterate listOfcards manualy to get all cards for current player. However, this very basic example allows you to add a card to the list at any time in your programm, and delete a card from the list at any time in the programm.

Answer 4

int printCards(struct Card *cardDeck)
{
    int sum = 0;
    static int i = 0;

    if(i < DECK_SIZE)
    {   
        switch (cardDeck[i].value + 1)
        {...}
        printf("of ");
        switch (cardDeck[i].suit)
        {...}

        sum += cardDeck[i].value;
        i++;

        return sum;
    }else
        printf("There are no cards left");
        return 0;
    }
}

use a static variable i: static int i=0; and after you go through the function i++;

and if you reach i = 51 you can return that there are no cards left any more!

I hope i could help you with that! best luck with your project :)