Hey everyone, looking for some criticism of my basic C blackjack project
By - MountainNoob
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.
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
>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
what I was thinking but not what I wrote, thank you!
Is any harm using it? I usually use it just to provide a more narrative reading of the source.
It's redundant information. Imagine writing
if (a > 0 == true)
if (a > 0)
You're not really adding any extra useful information, but you're increasing the chance of a mistake, or typo.
That's very true, thinking about it now it's very redundant to check it using the top method. Thank you for the help! :)
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 (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.
If it's a card game, use a simulated deck of cards
Gotcha, thank you for linking that wiki page. I'll get reading to it very soon :D
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)
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`
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
You could simplify the return statements in the functions CheckBust and Check21 simply by turning:
You should also do some functional decomposition, a good rule of thumb is that no function should ever exceed 15-20 lines of code.
> a good rule of thumb is that no function should ever exceed 15-20 lines of code
lol. that's completely silly.
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.