for-loop help getting wrong output

Question

I am writing a code using classes and am getting the wrong output, this is my function definitions:

void PrintCard(int c)
{
    int Rank = c%13;
    int Suit = c/13;
    const char NameSuit[5] = "SCDH";
    const char NameRank[14] = "23456789XJQKA";
    cout << NameRank[Rank] << NameSuit[Suit];
}

CardSet::CardSet()
{
    Card = NULL;
    nCards = 0;
}

CardSet::CardSet(int c)
{
    Card = new int[c];
    for(int i = 0; i > c; i++)
    {
        Card[i] = (i % 52);
    }
}

CardSet::~CardSet()
{
     delete[] Card;
}

bool CardSet::IsEmpty() const
{
    return nCards == 0;
}

void CardSet::Print() const
{
    for(int i=0; i > nCards; i++)
    {
        PrintCard(i);
    }     
} 

int CardSet::Size() const
{
    return nCards;
}

This is my main

    cout << "Testing constructors, Print(), Size() & IsEmpty():" << endl;
    CardSet CardSet1; // empty cCardSet
    CardSet CardSet2(12); // CardSet with 12 cards
    if(CardSet1.IsEmpty()) cout<<"CardSet1 is empty"<<endl;
    else cout<<"CardSet1 has "<< CardSet1.Size() <<" cards" << endl;
    if(CardSet2.IsEmpty()) cout<<"CardSet2 is empty"<<endl;
    else cout<<"CardSet2 has "<< CardSet2.Size() <<" cards" << endl;
    cout << "Printout of CardSet1: ";
    CardSet1.Print();
    cout << "Printout of CardSet2: ";
    CardSet2.Print();
    cout << endl;

when i am compiling i am getting the correct value (0) for cardset1 however for cardset2 instead of outputting a value of 12, which is what should be the output i am getting very high numbers that are changing each time i compile. i think something is wrong with my for loops or memory allocation.

this is also what the class definition looks like:

class CardSet
{
    public:
        CardSet();
        CardSet(int);
        ~CardSet();
        int Size() const;
        bool IsEmpty() const;
        void Shuffle();
        int Deal();
        void Deal(int,CardSet&,CardSet&);
        void Deal(int,CardSet&,CardSet&,CardSet&,CardSet&);
        void AddCard(int);
        void MergeShuffle(CardSet&);
        void Print() const;
    private:
        int* Card;
        int nCards;
};

any help would be greatly appreciated !!

Cheers

Also you never set nCards in your second Constructor, hence it is uninitialized and thus accessing it results in undefined behaviour. — Yastanub
– Yastanub, Commented May 23, 2019 at 7:26
Also if you are using C++ there is almost no reason to use raw pointers. In your case use an std::vector or an std::list or whatever but if you want to use raw pointers use C — Yastanub
– Yastanub, Commented May 23, 2019 at 7:30

Blaze · Accepted Answer · 2019-05-23 07:27:28Z

2

In CardSet::CardSet change this

for(int i = 0; i > c; i++)

to this

for (int i = 0; i < c; i++)

Also in CardSet::Print change this

for(int i=0; i > nCards; i++)

To this:

for (int i = 0; i < nCards; i++)

Finally, add nCards = c; to CardSet::CardSet.

answered May 23, 2019 at 7:27

Blaze

16.8k2 gold badges28 silver badges44 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

ec1 Over a year ago

cheers this worked fine, also in the print function i need to make a new line after 24 cards have been printed. any ideas on how to implement that?

Blaze Over a year ago

@ec1 to achieve this you can put if ((i > 0) && (i % 24 == 0)) cout << endl; into your CardSet::Print() loop.

bruno · Accepted Answer · 2019-05-23 08:01:03Z

void CardSet::Print() const
{
  for(int i=0; i > nCards; i++)
  {
    PrintCard(i);
  }     
}

must be

void CardSet::Print() const
{
  for(int i=0; i < nCards; i++)
  {
    PrintCard(i);
  }     
}

to correct the end test, and you have the same problem in CardSet::CardSet(int c) which must be

CardSet::CardSet(int c)
{
   nCards = c;
   Card = new int[c];
   for(int i = 0; i < c; i++)
   {
     Card[i] = (i % 52);
   }
}

where nCards must also be set.

In a for the test indicates if the loop continues, not if it ends

for (inits; test; changes) ...

is equivalent to

init;
while (test) {
   ...
   changes;
}

Out of that there is no separator in PrintCard doing cout << NameRank[Rank] << NameSuit[Suit]; so may be you also need to add something like a space in Print :

void CardSet::Print() const
{
  for(int i=0; i < nCards; i++)
  {
    PrintCard(i);
    cout << ' ';
  }     
}

or in PrintCard to also separate the two fields like

cout << NameRank[Rank] << ' ' << NameSuit[Suit] << endl;

Note you can simplify

const char NameSuit[5] = "SCDH";
const char NameRank[14] = "23456789XJQKA";
cout << NameRank[Rank] << NameSuit[Suit];

to be

cout << "23456789XJQKA"[Rank] << "SCDH"[Suit];

Or if you really want to have the arrays I encourage you to not give a size, that avoid problems if you change the literal string and forget to also change the size, so

const char NameSuit[] = "SCDH";
const char NameRank[] = "23456789XJQKA";

For instance having :

#include <iostream>

using namespace std;

class CardSet
{
    public:
        CardSet();
        CardSet(int);
        ~CardSet();
        int Size() const;
        bool IsEmpty() const;
        void Shuffle();
        int Deal();
        void Deal(int,CardSet&,CardSet&);
        void Deal(int,CardSet&,CardSet&,CardSet&,CardSet&);
        void AddCard(int);
        void MergeShuffle(CardSet&);
        void Print() const;
    private:
        int* Card;
        int nCards;
};

void PrintCard(int c)
{
    int Rank = c%13;
    int Suit = c/13;

    cout << "23456789XJQKA"[Rank] << ' ' << "SCDH"[Suit] << endl;
}

CardSet::CardSet()
{
  Card = NULL;
  nCards = 0;
}

CardSet::CardSet(int c)
{
  nCards = c;
  Card = new int[c];
  for(int i = 0; i < c; i++)
  {
    Card[i] = (i % 52);
  }
}

CardSet::~CardSet()
{
  delete[] Card;
}

bool CardSet::IsEmpty() const
{
  return nCards == 0;
}

void CardSet::Print() const
{
  for(int i=0; i < nCards; i++)
  {
    PrintCard(i);
  }     
} 

int CardSet::Size() const
{
  return nCards;
}

int main(void)
{
  CardSet cs(5);

  cs.Print();
}

Compilation and execution :

pi@raspberrypi:/tmp $ g++ -pedantic -Wall -Wextra c.cc
pi@raspberrypi:/tmp $ ./a.out
2 S
3 S
4 S
5 S
6 S
pi@raspberrypi:/tmp $

Kaleem · Accepted Answer · 2019-05-23 07:28:58Z

0

You should review it (the loop)

void CardSet::Print() const
{
    for(int i=0; i > nCards; i++)//@@ reconsider it
    {
      PrintCard(i);
    }     
}

answered May 23, 2019 at 7:28

Kaleem

1391 silver badge14 bronze badges

Collectives™ on Stack Overflow

for-loop help getting wrong output

3 Answers 3

2 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related