T O P

Hey everyone, looking for some criticism of my basic C blackjack project

Hey everyone, looking for some criticism of my basic C blackjack project

Mattlonn

got a few tips from a fast look isBust = CheckBust(cardSum, dealerSum); if (isBust == true) your function already return a boolean that you could use directly in your if . if (CheckBust(cardSum, dealerSum) also you dont need to check your boolean unless you want inverted logic. if(isBust) Also look up **macro** in c. Then you will have a easier understandable code and skip "loose numbers" like this: cardSum = cardSum + rand() % (10 + 1 - 1) + 1; //generating a random number between a certain n number //rand() % (max_number + 1 - minimum_number) + minimum_number


irqlnotdispatchlevel

>also you dont need to check your boolean unless you want inverted logic. I'd say that you should never compare a Boolean with true or false. If you want inverted logic just do if (!isBust)


Mattlonn

what I was thinking but not what I wrote, thank you!


bleuge

Is any harm using it? I usually use it just to provide a more narrative reading of the source.


irqlnotdispatchlevel

It's redundant information. Imagine writing if (a > 0 == true) Instead of if (a > 0) You're not really adding any extra useful information, but you're increasing the chance of a mistake, or typo.


MountainNoob

That's very true, thinking about it now it's very redundant to check it using the top method. Thank you for the help! :)


irqlnotdispatchlevel

Note that sometimes you may want to still wrap one liners into a macro or function. This is more of a style issue and different people will have different opinions, but code like: if (is_over_limit(value)) Or: if (value > FOO_LIMIT) Is easier to understand than: if (value > 205) In general, try to avoid magic numbers. Tools like cppcheck (understands C even if it has "CPP" in the name) and clang-tidy are good at spotting some of these issues.


oh5nxo

If it's a card game, use a simulated deck of cards https://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle


MountainNoob

Gotcha, thank you for linking that wiki page. I'll get reading to it very soon :D


elpaco555

Nice! 1) You check the same condition twice: `if (cardSum > dealerSum)` and `else if (cardSum > dealerSum)` 2) Maybe move `rand() % (10 + 1 - 1) + 1` to a function `GetCard()` 3) Don't check against `true` 4) `CheckBust` and `Check21` doesn't really use the dealersum argument for anything useful (except printing it) so I would suggest removing that argument 5) Taste: `CheckBust` -> `IsBust`, `Check21` -> `Is21` so it more correspondes to the boolean return (or even better remove the function and put it in the if statement directly)


dvhh

Hello, Blackjack is somewhat more deterministic than your algorithm would should, as the cards are dealt from a number of 52 cards decks, which would allow the player to estimate the probability of getting a card of a certain value. dealer and players pick from the same stack of cards. reference : [https://bicyclecards.com/how-to-play/blackjack/](https://bicyclecards.com/how-to-play/blackjack/) Now for the code : * why `(10 + 1 -1)` the result would be 10 whatever is the execution path * avoid test like `if(boolResult == true)` as you can write `if(boolResult)` * `fgetc` is probably more appropriate than `scanf` to read one char * check scanf return value * test for `cardSum > dealerSum` appear twice in the test list * considering that end conditions are somewhat exclusive I would write it differently 1. dealerSum > 21 : for a win condition 2. cardSum > dealerSum : for a win condition 3. cardSum < dealerSum : for a lose condition 4. cardSum == dealerSum : for a tie (no need to write the test as the else would be implicit) * for `CheckBust` and `Check21`, the `return false` could be written outside of the else block, as no extra code would be executed after `return` More on the stylistic aspect * `checkBust` should probably not print the message and that comment would nullify the reason for its existence * same for `Check21`


MountainNoob

Thank you for the very indepth tips. Also fgetc is such a good function idk why I didn't think of it before haha. I am forever grateful :P


rasqall

You could simplify the return statements in the functions CheckBust and Check21 simply by turning: else { return false; } into return false; You should also do some functional decomposition, a good rule of thumb is that no function should ever exceed 15-20 lines of code.


bonqen

> a good rule of thumb is that no function should ever exceed 15-20 lines of code lol. that's completely silly.


steamed_water

I think a better rule of thumb is simply "a function should do one thing". If a part of the operations that make up that thing is used somewhere else, then that part should be refactored as a separate function. However, a line count tends to fracture code amongst several locations, making it harder to change in the future.