Problem with nested class or array overrun.

[J. Campbell]

I'm trying to make a class that represents a deck of cards and a
dealer for use in a card game. I'm fairly novice. The code compiles
(Dev c++(gcc 3.3.1)) without complaint, however it acts strange when I
run it. (In windows), when I double-click or run the prog from the
IDE, it displays the deck, then exits. When I run it from a console
window, if I type the filename eg "cards.exe" , it displays the cards
as expected but doesn't pause at the end of the program. However if
I type "cards" without the explicit exe extension, the cards are not
displayed and I get the message:
"This application has requested the Runtime to terminate it in an
unusual way.
Please contact the application's support team for more information."

Any help is greatly appreciated. Joe
_______________ _______________ ___________
#include <iostream>
#include <stdint.h>
#include <string>

using namespace std;

struct acard{
int suit;
int value;
uint32_t index; // to be used by shuffle function
};

class deck{
private:
string theSuits; // contains suits in order
string theValues; // contains values in order
acard* card; // an array of cards
int numcards;

public:
deck();
~deck();
void show();
// void shuffle();
};

deck::deck():
theSuits("\x05\ x04\x03\x06") //c, d, h, s
,theValues("234 56789TJKQA")
,numcards(theSu its.length() * theValues.lengt h())
,card(new acard[numcards])

{
int cardnum(0);
for(int i = 0; i < theSuits.length (); ++i){
for(int j = 0; j < (theValues.leng th()); ++j){
cardnum = ((i * theValues.lengt h()) + j);
card[cardnum].suit = i;
card[cardnum].value = j;
}
}
}

deck::~deck(){
delete [] card;
}

void deck::show(){
for (int i=0; i < numcards; ++i){
cout << theValues[card[i].value]
<< theSuits[card[i].suit] << endl;
}
}


int main()
{
deck mydeck;
mydeck.show();

system("pause") ;
return 0;
}
_______________ _______________ ______

[Victor Bazarov]

Re: Problem with nested class or array overrun.

J. Campbell wrote:[color=blue]
> I'm trying to make a class that represents a deck of cards and a
> dealer for use in a card game. I'm fairly novice. The code compiles
> (Dev c++(gcc 3.3.1)) without complaint, however it acts strange when I
> run it. (In windows), when I double-click or run the prog from the
> IDE, it displays the deck, then exits. When I run it from a console
> window, if I type the filename eg "cards.exe" , it displays the cards
> as expected but doesn't pause at the end of the program. However if
> I type "cards" without the explicit exe extension, the cards are not
> displayed and I get the message:
> "This application has requested the Runtime to terminate it in an
> unusual way.
> Please contact the application's support team for more information."
>
> Any help is greatly appreciated. Joe[/color]

Regarding the message you get from your OS, I think you should ask in the
newsgroup for that OS. Regarding the code, see below.
[color=blue]
> _______________ _______________ ___________
> #include <iostream>
> #include <stdint.h>[/color]

No such standard header. Yet.
[color=blue]
> #include <string>
>
> using namespace std;
>
> struct acard{
> int suit;
> int value;
> uint32_t index; // to be used by shuffle function[/color]

No such standard type. Yet. Use at your own risk.
[color=blue]
> };
>
> class deck{
> private:
> string theSuits; // contains suits in order
> string theValues; // contains values in order
> acard* card; // an array of cards
> int numcards;[/color]

Swap the two lines before above. The explanation is in my next
paragraph.
[color=blue]
>
> public:
> deck();
> ~deck();
> void show();
> // void shuffle();
> };
>
> deck::deck():
> theSuits("\x05\ x04\x03\x06") //c, d, h, s
> ,theValues("234 56789TJKQA")
> ,numcards(theSu its.length() * theValues.lengt h())
> ,card(new acard[numcards])[/color]

This is very wrong. The initialisation happens in the order of the
declarations, not in the order you wrote in the initialiser list. So,
when you initialise the "card" member, "numcards" still contains trash.
What you allocate is impossible to say. Move the declaration of the
'numcards' member _before_ the 'card' member.
[color=blue]
>
> {
> int cardnum(0);
> for(int i = 0; i < theSuits.length (); ++i){
> for(int j = 0; j < (theValues.leng th()); ++j){
> cardnum = ((i * theValues.lengt h()) + j);[/color]

You might want to reconsider the use of superfluous parentheses. It
makes the code harder to read.
[color=blue]
> card[cardnum].suit = i;
> card[cardnum].value = j;
> }
> }
> }
>
> deck::~deck(){
> delete [] card;
> }
>
> void deck::show(){
> for (int i=0; i < numcards; ++i){
> cout << theValues[card[i].value]
> << theSuits[card[i].suit] << endl;
> }
> }
>
>
> int main()
> {
> deck mydeck;
> mydeck.show();
>
> system("pause") ;[/color]

The behaviour of 'system' is implementation-defined. There is no way
to predict what it will do. If you simply need to pause your program,
you might want to use getchar().
[color=blue]
> return 0;
> }[/color]


Victor

[Joe C]

Re: Problem with nested class or array overrun.


"Victor Bazarov" <v.Abazarov@com Acast.net> wrote in message
news:TyuSc.674$ 191.330@newsrea d1.dllstx09.us. to.verio.net...
[color=blue][color=green]
> > class deck{
> > private:
> > string theSuits; // contains suits in order
> > string theValues; // contains values in order
> > acard* card; // an array of cards
> > int numcards;[/color]
>
> Swap the two lines before above. The explanation is in my next
> paragraph.
>[color=green]
> >
> > public:
> > deck();
> > ~deck();
> > void show();
> > // void shuffle();
> > };
> >
> > deck::deck():
> > theSuits("\x05\ x04\x03\x06") //c, d, h, s
> > ,theValues("234 56789TJKQA")
> > ,numcards(theSu its.length() * theValues.lengt h())
> > ,card(new acard[numcards])[/color]
>
> This is very wrong. The initialisation happens in the order of the
> declarations, not in the order you wrote in the initialiser list. So,
> when you initialise the "card" member, "numcards" still contains trash.
> What you allocate is impossible to say. Move the declaration of the
> 'numcards' member _before_ the 'card' member.
>
> Victor[/color]

Thanks Victor. What a critical mistake! I confess ignorance. I thought,
as you guessed, that the order was as-per the initialiser list...

I included the compiler info to cover bases, since I couldn't figure it out
what was going wrong...and the OS-specific call, because...well. ..I won't
try to justify it...

Thanks again for the real help...this detail will carry forward. I wish I
could pay you back. Your helping karma is golden.

Joe

[Joe C]

Re: Problem with nested class or array overrun.

One detail I forgot to ask. Suppose I have a class where the initialization
order is critical, and that the order alternates between public and private.
Does it become necessarry to do something like this?:

class myclass{
public:
int ImFirst;
private:
int ImSecond;
public:
int ImThird;
private:
int ImFourth;
};

Thanks again.

[Victor Bazarov]

Re: Problem with nested class or array overrun.

Joe C wrote:[color=blue]
> One detail I forgot to ask. Suppose I have a class where the initialization
> order is critical, and that the order alternates between public and private.
> Does it become necessarry to do something like this?:
>
> class myclass{
> public:
> int ImFirst;
> private:
> int ImSecond;
> public:
> int ImThird;
> private:
> int ImFourth;
> };[/color]

I've never seen anything like this, but I suppose if the requirement is so
unusual, so should be the syntax.

Let me just note here that if the order of initialisation is important to
the point when you have to introduce some weirdness to the class, you may
need to rethink your design, reconsider the reasons according to which the
order of initialisation has to be specific. And also, if you ever need
the data members to be public, you should also think twice. I cannot
remember ever having to have both private and public data in the same
class. The data are either all public or all private (and much more often
the latter than the former).

Victor

[Old Wolf]

Re: Problem with nested class or array overrun.

mango_maniac@ya hoo.com (J. Campbell) wrote:
[color=blue]
> I'm trying to make a class that represents a deck of cards and a
> dealer for use in a card game. I'm fairly novice.[/color]

First lesson... don't manually manage memory when there's an easier way.
[color=blue]
> #include <iostream>
> #include <stdint.h>
> #include <string>[/color]

#include <vector>
[color=blue]
>
> using namespace std;
>
> struct acard{
> int suit;
> int value;
> uint32_t index; // to be used by shuffle function
> };
>
> class deck{
> private:
> string theSuits; // contains suits in order
> string theValues; // contains values in order[/color]

Are the suits and values ever going to change during a game? If not,
then use:
string const theSuits;
etc.

Are they specific to this deck? If all decks will have the same suit
and values then you should either make these items static, or remove
them from the class entirely. Also it is considered good design
these days to have code that populates the deck, not actually as a
member function of the deck, ie. when you create a new deck, it
starts off blank and the constructor does as little as possible;
and then you have a free function that takes a deck as reference
parameter and adds all the cards to it.
[color=blue]
> acard* card; // an array of cards
> int numcards;[/color]

Replace these two with:
vector<acard> card;[color=blue]
>
> public:
> deck();
> ~deck();
> void show();
> // void shuffle();[/color]

Use std::random_shu ffle() instead of writing your own shuffle. Again
it would be good design for this to not be a member function.
[color=blue]
> };
>
> deck::deck():
> theSuits("\x05\ x04\x03\x06") //c, d, h, s
> ,theValues("234 56789TJKQA")[/color]
[color=blue]
> ,numcards(theSu its.length() * theValues.lengt h())
> ,card(new acard[numcards])[/color]

,acard(theSuits .length() * theValues.lengt h())
[color=blue]
> deck::~deck(){
> delete [] card;
> }[/color]

You don't need this function at all now. The rest of the program
stays the same

If you stuck with your pointer approach, then you would also have to
add a copy-constructor and an assignment operator.

[Victor Bazarov]

Re: Problem with nested class or array overrun.

Old Wolf wrote:[color=blue]
> mango_maniac@ya hoo.com (J. Campbell) wrote:
>
>[color=green]
>>I'm trying to make a class that represents a deck of cards and a
>>dealer for use in a card game. I'm fairly novice.[/color]
>
>
> First lesson... don't manually manage memory when there's an easier way.
>
>[color=green]
>>#include <iostream>
>>#include <stdint.h>
>>#include <string>[/color]
>
>
> #include <vector>
>[color=green]
>>using namespace std;
>>
>>struct acard{
>> int suit;
>> int value;
>> uint32_t index; // to be used by shuffle function
>>};
>>
>>class deck{
>> private:
>> string theSuits; // contains suits in order
>> string theValues; // contains values in order[/color]
>
>
> Are the suits and values ever going to change during a game? If not,
> then use:
> string const theSuits;
> etc.
>
> Are they specific to this deck? If all decks will have the same suit
> and values then you should either make these items static, or remove
> them from the class entirely. Also it is considered good design
> these days to have code that populates the deck, not actually as a
> member function of the deck, ie. when you create a new deck, it
> starts off blank and the constructor does as little as possible;
> and then you have a free function that takes a deck as reference
> parameter and adds all the cards to it.
>
>[color=green]
>> acard* card; // an array of cards
>> int numcards;[/color]
>
>
> Replace these two with:
> vector<acard> card;
>[color=green]
>>
>> public:
>> deck();
>> ~deck();
>> void show();
>>// void shuffle();[/color]
>
>
> Use std::random_shu ffle() instead of writing your own shuffle. Again
> it would be good design for this to not be a member function.
>
>[color=green]
>>};
>>
>>deck::deck( ):
>> theSuits("\x05\ x04\x03\x06") //c, d, h, s
>> ,theValues("234 56789TJKQA")[/color]
>
>[color=green]
>> ,numcards(theSu its.length() * theValues.lengt h())[/color][/color]

This is not needed if 'numcards' is not there any more, right?
[color=blue][color=green]
>> ,card(new acard[numcards])[/color]
>
>
> ,acard(theSuits .length() * theValues.lengt h())[/color]

Actually

, card(theSuits.l ength() * theValues.lengt h())
[color=blue]
>
>[color=green]
>>deck::~deck() {
>> delete [] card;
>>}[/color]
>
>
> You don't need this function at all now. The rest of the program
> stays the same
>
> If you stuck with your pointer approach, then you would also have to
> add a copy-constructor and an assignment operator.[/color]

How true...

V

[Old Wolf]

Re: Problem with nested class or array overrun.

Victor Bazarov <v.Abazarov@com Acast.net> wrote:[color=blue]
> Old Wolf wrote:[color=green]
> > mango_maniac@ya hoo.com (J. Campbell) wrote:[color=darkred]
> >> ,card(new acard[numcards])[/color]
> > ,acard(theSuits .length() * theValues.lengt h())[/color]
>
> Actually
> , card(theSuits.l ength() * theValues.lengt h())[/color]

Right. All the more reason to use a better naming convention than
"acard" as a typename and "card" as an acard.