Problem with nested class or array overrun.

Discussion in 'C++' started by J. Campbell, Aug 11, 2004.

  1. J. Campbell

    J. Campbell Guest

    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("23456789TJKQA")
    ,numcards(theSuits.length() * theValues.length())
    ,card(new acard[numcards])

    {
    int cardnum(0);
    for(int i = 0; i < theSuits.length(); ++i){
    for(int j = 0; j < (theValues.length()); ++j){
    cardnum = ((i * theValues.length()) + 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.value]
    << theSuits[card.suit] << endl;
    }
    }


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

    system("pause");
    return 0;
    }
    ____________________________________
     
    J. Campbell, Aug 11, 2004
    #1
    1. Advertising

  2. J. Campbell wrote:
    > 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


    Regarding the message you get from your OS, I think you should ask in the
    newsgroup for that OS. Regarding the code, see below.

    > _________________________________________
    > #include <iostream>
    > #include <stdint.h>


    No such standard header. Yet.

    > #include <string>
    >
    > using namespace std;
    >
    > struct acard{
    > int suit;
    > int value;
    > uint32_t index; // to be used by shuffle function


    No such standard type. Yet. Use at your own risk.

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


    Swap the two lines before above. The explanation is in my next
    paragraph.

    >
    > public:
    > deck();
    > ~deck();
    > void show();
    > // void shuffle();
    > };
    >
    > deck::deck():
    > theSuits("\x05\x04\x03\x06") //c, d, h, s
    > ,theValues("23456789TJKQA")
    > ,numcards(theSuits.length() * theValues.length())
    > ,card(new acard[numcards])


    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.

    >
    > {
    > int cardnum(0);
    > for(int i = 0; i < theSuits.length(); ++i){
    > for(int j = 0; j < (theValues.length()); ++j){
    > cardnum = ((i * theValues.length()) + j);


    You might want to reconsider the use of superfluous parentheses. It
    makes the code harder to read.

    > 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.value]
    > << theSuits[card.suit] << endl;
    > }
    > }
    >
    >
    > int main()
    > {
    > deck mydeck;
    > mydeck.show();
    >
    > system("pause");


    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().

    > return 0;
    > }



    Victor
     
    Victor Bazarov, Aug 11, 2004
    #2
    1. Advertising

  3. J. Campbell

    Joe C Guest

    "Victor Bazarov" <> wrote in message
    news:TyuSc.674$09.us.to.verio.net...

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

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

    >
    > 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


    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, Aug 11, 2004
    #3
  4. J. Campbell

    Joe C Guest

    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.
     
    Joe C, Aug 11, 2004
    #4
  5. Joe C wrote:
    > 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;
    > };


    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
     
    Victor Bazarov, Aug 11, 2004
    #5
  6. J. Campbell

    Old Wolf Guest

    (J. Campbell) wrote:

    > 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.


    First lesson... don't manually manage memory when there's an easier way.

    > #include <iostream>
    > #include <stdint.h>
    > #include <string>


    #include <vector>

    >
    > 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


    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.

    > acard* card; // an array of cards
    > int numcards;


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


    Use std::random_shuffle() instead of writing your own shuffle. Again
    it would be good design for this to not be a member function.

    > };
    >
    > deck::deck():
    > theSuits("\x05\x04\x03\x06") //c, d, h, s
    > ,theValues("23456789TJKQA")


    > ,numcards(theSuits.length() * theValues.length())
    > ,card(new acard[numcards])


    ,acard(theSuits.length() * theValues.length())

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


    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.
     
    Old Wolf, Aug 12, 2004
    #6
  7. Old Wolf wrote:
    > (J. Campbell) wrote:
    >
    >
    >>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.

    >
    >
    > First lesson... don't manually manage memory when there's an easier way.
    >
    >
    >>#include <iostream>
    >>#include <stdint.h>
    >>#include <string>

    >
    >
    > #include <vector>
    >
    >>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

    >
    >
    > 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.
    >
    >
    >> acard* card; // an array of cards
    >> int numcards;

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

    >
    >
    > Use std::random_shuffle() instead of writing your own shuffle. Again
    > it would be good design for this to not be a member function.
    >
    >
    >>};
    >>
    >>deck::deck():
    >> theSuits("\x05\x04\x03\x06") //c, d, h, s
    >> ,theValues("23456789TJKQA")

    >
    >
    >> ,numcards(theSuits.length() * theValues.length())


    This is not needed if 'numcards' is not there any more, right?

    >> ,card(new acard[numcards])

    >
    >
    > ,acard(theSuits.length() * theValues.length())


    Actually

    , card(theSuits.length() * theValues.length())

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

    >
    >
    > 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.


    How true...

    V
     
    Victor Bazarov, Aug 12, 2004
    #7
  8. J. Campbell

    Old Wolf Guest

    Victor Bazarov <> wrote:
    > Old Wolf wrote:
    > > (J. Campbell) wrote:
    > >> ,card(new acard[numcards])

    > > ,acard(theSuits.length() * theValues.length())

    >
    > Actually
    > , card(theSuits.length() * theValues.length())


    Right. All the more reason to use a better naming convention than
    "acard" as a typename and "card" as an acard.
     
    Old Wolf, Aug 17, 2004
    #8
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. spoc
    Replies:
    8
    Views:
    574
    Old Wolf
    Sep 9, 2004
  2. Anthony Baxter
    Replies:
    0
    Views:
    272
    Anthony Baxter
    Oct 12, 2006
  3. Martin Eisenberg

    Buffer overrun - exit or abort?

    Martin Eisenberg, Jan 16, 2006, in forum: C++
    Replies:
    8
    Views:
    450
    Earl Purple
    Jan 18, 2006
  4. Nico
    Replies:
    2
    Views:
    307
    Salt_Peter
    Oct 15, 2006
  5. =?Utf-8?B?TGl0dGxlbXV6?=

    buffer overrun

    =?Utf-8?B?TGl0dGxlbXV6?=, Aug 27, 2007, in forum: ASP .Net
    Replies:
    3
    Views:
    1,890
    =?Utf-8?B?SXBvZHVzZXI=?=
    Nov 11, 2007
Loading...

Share This Page