Bitwise Unwanted Output

Hi guys, so I'm writing a program that I can later implement into maybe a blackjack game or something if I want to. Anyways it's supposed to create a deck of cards, and each card is stored in 1 byte (each element of a 52 byte character array should store a card number/suit). I'm using bitwise operators etc to do this.

Anyways I'm not receiving the output that I want. The output that I should be getting (if it worked properly) would be an even amount of suits (13 of each suit). Instead I'm getting 5 clubs, 16 diamonds, 15 hearts, 16 spades. I'm pretty certain it's a flaw in my constructor, which is modifying the binary in each byte of the array.

Here is the output that I get when I run the program:

Code:

Card 0: 00000000 suit: Club
Card 1: 00010001 suit: Diamond
Card 2: 00100010 suit: Heart
Card 3: 00110011 suit: Spade
Card 4: 00000100 suit: Club
Card 5: 00010101 suit: Diamond
Card 6: 00100110 suit: Heart
Card 7: 00000111 suit: Club
Card 8: 00011000 suit: Diamond
Card 9: 00101001 suit: Heart
Card 10: 00001010 suit: Club
Card 11: 00011011 suit: Diamond
Card 12: 00101100 suit: Heart
Card 13: 00001101 suit: Club
Card 14: 00011110 suit: Diamond
Card 15: 00101111 suit: Heart
Card 16: 00010000 suit: Diamond
Card 17: 00010001 suit: Diamond
Card 18: 00110010 suit: Spade
Card 19: 00010011 suit: Diamond
Card 20: 00010100 suit: Diamond
Card 21: 00110101 suit: Spade
Card 22: 00010110 suit: Diamond
Card 23: 00010111 suit: Diamond
Card 24: 00111000 suit: Spade
Card 25: 00011001 suit: Diamond
Card 26: 00011010 suit: Diamond
Card 27: 00111011 suit: Spade
Card 28: 00011100 suit: Diamond
Card 29: 00011101 suit: Diamond
Card 30: 00111110 suit: Spade
Card 31: 00011111 suit: Diamond
Card 32: 00110000 suit: Spade
Card 33: 00100001 suit: Heart
Card 34: 00100010 suit: Heart
Card 35: 00110011 suit: Spade
Card 36: 00100100 suit: Heart
Card 37: 00100101 suit: Heart
Card 38: 00110110 suit: Spade
Card 39: 00100111 suit: Heart
Card 40: 00101000 suit: Heart
Card 41: 00111001 suit: Spade
Card 42: 00101010 suit: Heart
Card 43: 00101011 suit: Heart
Card 44: 00111100 suit: Spade
Card 45: 00101101 suit: Heart
Card 46: 00101110 suit: Heart
Card 47: 00111111 suit: Spade
Card 48: 00110000 suit: Spade
Card 49: 00110001 suit: Spade
Card 50: 00110010 suit: Spade
Card 51: 00110011 suit: Spade

As you can see, that's not right.. It is SUPPOSED to be Club, Diamond, Heart, Spade, in that order for every card. So ovbiously there is something wrong with the method I'm using in the constructor Here is the code:

Code:

#include <iostream>
#include <cstdlib>
 
using namespace std;
 
void outputBinary(char value);
void checkSuit(char value);
 
class CardDeck {
public:
	CardDeck();
	~CardDeck();
	unsigned char * cards;
};
 
CardDeck::CardDeck() {
    int i = 0;
	int x = 0;
	int count = 0;
	this->cards = new unsigned char[52];
 
	for (int i = 0; i < 52; count++) {
	this->cards[i] = i++ | x;
	x+=16;
		if (count == 3) { 
			x = 0;
			count = 0;
		}
	}
}
 
CardDeck::~CardDeck() {
	delete [] this->cards;
}
 
int main() {
	CardDeck c;
	for (int x = 0; x < 52; x++) {
		cout << endl << "Card " << x << ": ";
		outputBinary(c.cards[x]);
		cout << " suit: ";
		checkSuit(c.cards[x]);
	}
}
 
void checkSuit(char value) {
 
	unsigned char x = 0x30 & value;
 
	switch ( x ) {
 
		case 0x30:
			cout << "Spade";
			break;
		case 0x20:
			cout << "Heart";
			break;
		case 0x10:
			cout << "Diamond";
			break;
		case 0x0:
			cout << "Club";
			break;
		default:
			cout << "No match found.";
			break;
	}
}

I receive no compiler errors or warnings, and I'm using visual studio C++ in case you're wondering. Thanks!

You can be quite sure that this line is undefined behaviour.

Code:

this->cards[i] = i++ | x;

If you are performing an operation on a variable that has side effects on the variable (such as ++), it shouldn't appear more than once in the statement. In this case it is undefined whether the old or new value of i should be used for the index.

Another thing is, since you are or-ing numbers, it might mess up the two x bits which are supposed to be the suit? Perhaps you need some bit-shifting so that the bit pattern of i doesn't overlap that of x?

Another thing is what x does in the loop. It will start from 0, but later it will always go 1, 2, 3 because it will be incremented by the for loop. If that is not intended perhaps increment i in the loop and handle incrementing x in the loop yourself.

Here's my solution:

Code:

CardDeck::CardDeck() {

	this->cards = new unsigned char[52];
	int n = 0;

	for (int i = 0; i < 0xD; i++) // 0xD = 13
		for (int j = 0; j < 4; j++)
			this->cards[n++] = ((j << 4) | i);
}

By the way, if you try to copy your CardDeck class it will cause problems (probably a crash). Is there a reason you need a dynamic array? Why not make the array statically sized at 52. If you want the flexibility of the dynamic array, use a vector instead.

If you have to continue with the raw dynamic array, disable copying or implement the rule of three properly.

Code:

	for (int i = 0; i < 52; count++) {
	this->cards[i] = i++ | x;
	x+=16;
		if (count == 3) { 
			x = 0;
			count = 0;
		}
	}

could be better implemented using bitwise AND to avoid the if-statement (and thus run about 3x faster due to the lack of "random" ("random" with regards to branch prediction that is).

For example:

Code:

	for (int i = 0; i < 52; i++) {
          this->cards[i] = i | (count << 4);
          count++;
          count &= 3;
	}

or

Code:

	for (int i = 0; i < 52; i++) {
          this->cards[i] = i | (count << 4);
          count+=16;
          count &= 48;
	}

--
Mats