Thread: Bitwise Unwanted Output

  1. #1
    Registered User
    Join Date
    May 2008
    Location
    Australia
    Posts
    230

    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!
    Last edited by pobri19; 09-15-2008 at 01:35 AM.
    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.

  2. #2
    The larch
    Join Date
    May 2006
    Posts
    3,573
    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.
    Last edited by anon; 09-15-2008 at 02:24 AM.
    I might be wrong.

    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
    Quoted more than 1000 times (I hope).

  3. #3
    Registered User
    Join Date
    May 2008
    Location
    Australia
    Posts
    230
    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);
    }
    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.

  4. #4
    Registered User
    Join Date
    Jan 2005
    Posts
    7,366
    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.

  5. #5
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    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
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Help for my output array
    By qwertysingh in forum C Programming
    Replies: 1
    Last Post: 02-17-2009, 03:08 PM
  2. execl()/fork() output
    By tadams in forum C Programming
    Replies: 19
    Last Post: 02-04-2009, 03:29 PM
  3. Formatting output into even columns?
    By Uncle Rico in forum C Programming
    Replies: 2
    Last Post: 08-16-2005, 05:10 PM
  4. Output problems with structures
    By Gkitty in forum C Programming
    Replies: 1
    Last Post: 12-16-2002, 05:27 AM
  5. Simple File Creation Algorithm
    By muffin in forum C Programming
    Replies: 13
    Last Post: 08-24-2001, 03:28 PM