3
\$\begingroup\$

I just wanted to make a fun project for practise and as far as I know the program runs just fine without any flaws, but I would like to add an option to it, if a round ends to ask the user if he wants to play another round. Maybe I should put the whole thing in a while loop or something but everything I tried failed. And I should reset the first array int values_w_p[][2], but I don't know what would be the best method for that. Another thing would be of course the betting and balance, but for that I have at least some ideas which I will try. Tips are welcome too! Here is the code:

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

int *actValue(int values[][2]){     // function to create the array based on probability
    
    int len = 0;
    for (int i = 0; i<10;i++){
        len = len + values[i][1];
    }
    int *arr = (int*)malloc(len*(sizeof(int)));
    int count=0;
    for (int q = 0; q<10; q++){
        for (int w = 0; w < values[q][1]; w++){
            arr[count]=values[q][0];
            count++;
        }
    }
    return arr;

}

int lenOfArr(int values[][2]){  //based on the probability array returns the length of it (values_w_p)
    
    int len = 0;
    for (int i = 0; i<10;i++){
        len = len + values[i][1];
    }
    return len;
}

int drawacard(int *valuenow, int len, int values_w_p[][2]){      // picks a random value from the array generated with actValue and takes one from the probability 
    
    int yourcard = valuenow[rand() % (len - 0) + 0];
    values_w_p[yourcard-2][1] -= 1;
    return yourcard;
}

void name(int card) {       //prints the name of the card baed on the value

    const char *color [] = {"Clubs", "Diamonds", "Hearts", "Spades" };
    const char *nev [] = {"2","3","4","5","6","7","8","9","10","Jack", "Queen", "King","Ace" };

    if(card < 10){
        printf("%s of %s\n", nev[card-2], color[rand()%4]);
    }
    else if (card == 10){
        printf("%s of %s\n", nev[rand() % (11 + 1 - 8) + 8], color[rand()%4]);
    }
    else if(card == 11){
        printf("%s of %s\n", nev[12], color[rand()%4]);
    }
}

int total(int total[]){         //it will calc the total value of a the cards based on the array(player or dealer) 

    int sum = 0;
    for (int j = 0; j<16;j++){
        sum = sum + total[j];
    }
    return sum;
}

int main(){

    srand((unsigned int)time(0));

    int values_w_p [][2] = {  
        {2,8},{3,8},{4,8},{5,8},{6,8},{7,8},{8,8},{9,8},{10,32},{11,8}
    };
    int *ptotal = (int*)calloc(16,sizeof(int));
    int *dtotal = (int*)calloc(16,sizeof(int));
    ptotal[0]= drawacard(actValue(values_w_p), lenOfArr(values_w_p), values_w_p);

    ptotal[1]= drawacard(actValue(values_w_p), lenOfArr(values_w_p), values_w_p);

    dtotal[0]= drawacard(actValue(values_w_p), lenOfArr(values_w_p), values_w_p);

    printf("\nyour card: ");

    name(ptotal[0]);

    printf("\nyour card: ");

    name(ptotal[1]);

    printf("\ndealers card: ");

    name(dtotal[0]);

    printf("\n------------------------------------------------\n");

    int inp=0;
    int count=0;
    while(inp==0){
        if (total(ptotal)==21){ 
            printf("\nYou have a blackjack, lets see the dealers hand");
            dtotal[count] = drawacard(actValue(values_w_p), lenOfArr(values_w_p), values_w_p);
            printf("\ndealers card: ");
            name(dtotal[count]);
            if (total(dtotal)!=21){ 
                printf("\nCongrats! You win!");
                break;
            }
            else{
                printf("\nSadly its a TIE");
                break;
            }
        }
        printf("Hit or stand?(0 for hit, 1 for stand): ");
        scanf("%d", &inp);
        if (inp == 1){
            while(total(dtotal)<17){
                count++;
                dtotal[count] = drawacard(actValue(values_w_p), lenOfArr(values_w_p),values_w_p);
                printf("\ndealers card: ");
                name(dtotal[count]);
                if (total(dtotal)==21){
                    printf("\nThe dealer has a blackjack, you lost");
                    break;
                }
                else if(total(dtotal)>21 && total(ptotal)<22){
                    printf("\nYou win");
                    break;
                }
                else if(total(dtotal)>21 && total(ptotal)>21){
                    printf("\n Its a TIE");
                    break;
                }
                else if(total(dtotal)>16 && total(dtotal)>total(ptotal)){
                    printf("You lost");
                    break;
                } 
                else if(total(dtotal)==total(ptotal) && total(dtotal)>16){
                    printf("its a TIE");
                    break;
                }
                else if(total(dtotal)<total(ptotal) && total(dtotal)>16){
                    printf("You win");
                    break;
                }
            }
        }
        else{
            count++;
            ptotal[count+1] = drawacard(actValue(values_w_p), lenOfArr(values_w_p), values_w_p);
            printf("\nyour card: ");
            name(ptotal[count+1]);
            if (total(ptotal)>21){
                printf("\nYou lost");
                break;
            }
        }
    }
    return 0;
}
\$\endgroup\$
1
  • \$\begingroup\$ Could you please have a second look at your formatting? I think some mistakes have been made, and the code is consequently quite difficult to read. \$\endgroup\$
    – ades
    Commented Dec 30, 2021 at 10:44

2 Answers 2

3
\$\begingroup\$

Allocate to the size of the referenced object, not type

  • Cast not needed in C.

  • sizeof *the_pointer is always the correct type. sizeof(int) relies on matching the type - which may change with updates and is harder to review when the type is far from the malloc() line of code.

  • Lead with sizeof ... to insure math used is at least size_t. Makes no difference in OP's code, yet consider size_t * int * int vs. int * int * size_t: which may unnecessarily overflow?

      // int *arr = (int*)malloc(len*(sizeof(int)));
      int *arr = malloc(sizeof *arr * len);
    

Robust code checks for allocation failure.

// When len is always > 0
if (arr == NULL) Handle_Failure();
// ... otherwise 
if (arr == NULL && len != 0) Handle_Failure();

Avoid naked magic numbers

What is the 10 all about? Where does it originate?

Better to pass in, as a size_t. Use size_t for sizing and array indexing. It is not too narrow nor too wide. size_t is an unsigned type, so code with that in mind.

//int *actValue(int values[][2]){
...
//  for (int i = 0; i<10;i++){

int *actValue(size_t size, int values[][2]) {
  ...
  for (size_t i = 0; i < size; i++) {

Suspicious magic number

What is the -2 in values_w_p[yourcard-2][1] -= 1; for? Looks like a bug.

Code could really use more comments

Many object names lack clarity like what is values_w_p?

In common blackjack an ace is 1 or 11

total() does not account for that. Count the aces and increase the score by 10 if not over 21.

Avoid long lines

Rather than

int drawacard(int *valuenow, int len, int values_w_p[][2]){      // picks a random value from the array generated with actValue and takes one from the probability 

... consider wrapping at with 80 or so.

// Picks a random value from the array generated with actValue and 
// takes one from the probability. 
int drawacard(int *valuenow, int len, int values_w_p[][2]) {

Use better card and blackjack terms

Suit works better than color here as there are usually 2 colores: red/black, but 4 suits.

// const char *color [] = {"Clubs", "Diamonds", "Hearts", "Spades" };
const char *suit[] = {"Clubs", "Diamonds", "Hearts", "Spades" };

Maybe rank vs nev.

// const char *nev [] = ....
const char *rank[] = ....

For a starting code task, OP has chosen a good one and made a good attempt.

I found it too unorganized though and recommend significant re-write.

Following are some untested pieces that OP may find useful.

// https://en.wikipedia.org/wiki/Glossary_of_blackjack_terms

// Number of suits
#define SUIT_N 4
// Number of ranks
#define RANK_N 13
#define RANK_ACE 0
// Number of decks
#define DECK_N 5
// Total cards
#define CARDS_N (SUIT_N * RANK_N * DECK_N)
// Max cards in a hand: 20 aces + 1 more
#define CARD_N 21

typedef struct {
  int rank;
  int suit;
} card;

typedef struct {
  card c[CARD_N];
  int count;
} hand;

typedef struct {
  card c[CARDS_N];
  int count;
} stock;

static const signed char rank_value[RANK_N] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10,
    10, 10, 10};

void shuffle(stock *st) {
  // https://en.wikipedia.org/wiki/Fisher–Yates_shuffle#The_modern_algorithm
  int index = 0;
  for (int deck = 0; deck < DECK_N; deck++) {
    for (int suit = 0; suit < SUIT_N; suit++) {
      for (int rank = 0; rank < RANK_N; rank++) {
        st->c[index++] = (card) {.rank = rank, .suit = suit};
      }
    }
  }
  st->count = index;
  while (--index > 0) {
    int j = rand() % (index + 1);
    card temp = st->c[index];
    st->c[index] = st->c[j];
    st->c[j] = temp;
  }
}

int hand_total(const hand *h) {
  int total = 0;
  bool ace_flag = false;
  for (int i = 0; i < h->count; i++) {
    total += rank_value[h->c[i].rank];
    if (h->c[i].rank == RANK_ACE) {
      ace_flag = true;
    }
  }
  if (ace_flag && total <= 11) {
    total += 10;
  }
  return total;
}

I'd say there is more for others to review.

\$\endgroup\$
2
  • \$\begingroup\$ First of all thank you for the really great feedback, and yes my code looks a bit messy. Some additions: int values_w_p [][2] stands for values with probability, and it represents two decks of cards. Every sub-array represents how many "values" are in the two decks. That's why I used values_w_p[yourcard-2][1] -= 1; because if you draw a card one card is out of the decks, probably it's not the best way to do that, but that's the best solution I came up with. Based on the array values_w_p int *actValue(int values[][2]) calculates "all of the cards." \$\endgroup\$
    – Maxell
    Commented Jan 2, 2022 at 23:50
  • 1
    \$\begingroup\$ @Maxell It is not that I am looking for a comment to my answer, it is that if your "stands for values with probability, and it represents two decks of cards." was a comment in your code near int values_w_p [][2] = { {2,8},..., then your code gain in clarity. Often a one line comment about a cryptic variable name is all that is needed. \$\endgroup\$
    – chux
    Commented Jan 3, 2022 at 6:07
1
\$\begingroup\$

Gameplay

It's pretty fun.

"0 for hit, 1 for stand," is fine in development, but awkward. I found myself thinking 'h' or 's' would be better. Reading a beginner's guide will probably save you some trouble with the traps of C I/O.

You should re-iterate the hand so far so I don't have to look back in my buffer and see. Adding up values in my head is fine in casinos, but maybe some help would be appreciated here?

"You lost," is kind of vague. I would expect at least a differential analysis of my hand vs the dealer's.

Code

You don't free any pointers, thus you have a memory leak. This is possibly not critical, but as your project develops, it's going to take more and more memory. This is a bounded game; at most, there are 52 cards; I think you should consider stack allocation as a simple alternative to malloc. Simple is good. (Note that planned expansions possibly would be good with dynamic memory.)

Comments are well thought out and readable, but the code is not self-documenting; it's very hard to follow the parameters, eg, int values[][2], int values_w_p[][2].

The card is sampled and replaced, but in a real game, it's not replaced. Usually one can get away with this simplification, but sometimes it's a problem, (five 2's?) The value needs some work: an ace can add only one 11, so you could do it in parallel sum or backtrack.

Use enums for improved readability

Instead of magic numbers, which are hard to follow, I think you could benefit by an enum. enum rank { ACE = 1, TWO, ..., KING }. Placing enum suit { SPADES, DIAMONDS, CLUBS, HEARTS } then would allow struct card { enum suit; enum rank; }.

Edit: how to stop memory leaks

Could you give an example, where should I free up the allocated memory?

There are three places when one allocates memory: what is returned from actValue, ptotal, and dtotal, (player hand and dealer hand, perhaps?) We would expect that there are three places you should be freeing memory, too. Presumably, when the object is not needed. Sometimes it can be tricky; a very useful tool for debugging memory allocations is valgrind.

However, these are finite sized, so you could very well not use dynamic memory and declare them int ptotal[16], dtotal[16];. Since actTotal is always going to called as an argument for drawcard and then, it appears, drawing one and then forgetting about the rest, it would be nice to have a static temporary card buffer (static int arr[MAX_VALUE];) instead of leaking memory every time, it will use the same static memory every time.

I'm not sure what lenofArr does, but usually you would have a size along with the array. In @chux review, they implemented a Fisher–Yates shuffle, which is probably what you want in a card game. This solves the problem of sampling without replacement and has a count built in. This is probably what I would go towards.

\$\endgroup\$
2
  • \$\begingroup\$ Thank you, I changed the end messages already and added a value counter, so you don't have to count them. I tried to use free() but it just messed up my program. Could you give an example, where should I free up the allocated memory? \$\endgroup\$
    – Maxell
    Commented Jan 2, 2022 at 23:55
  • \$\begingroup\$ In terms of gameplay, that's a lot more fun! I recommend using dynamic allocation sparingly; it will generally be faster. I had a lot to say, so I've edited my answer. \$\endgroup\$
    – Neil
    Commented Jan 3, 2022 at 19:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.