C++ Same output while using switch inside while loop

Question

I have tried to make the following test program work for 2 days now,however its not working. It is based on several header files which work completely fine, because I checked them via another testprogram. It has header files called Area,Circle,Ring,Rectangle and Square. Also I defined function randcolor and randsize; I checked everyhting over and over, however it is producing the same ouptut aftera second try in the while loop:

int main()
{
  srand(time(NULL));
  Area *list[20];
  int m;
  Area *c;
  int j = 0;

  while (j < 20) {

    m = rand() % 4;
    cout << m << endl;

    switch (m) {
      case 0: {
        Circle a(randcolor(), randsize());
        c = &a;
        break; 
      }
      case 1: {
        Ring r(randcolor(), randsize(), randsize());
        c = &r;
        break;
      }
      case 2: {
        Rectangle re(randcolor(), randsize(), randsize());
        c = &re;
        break;
      }
      case 3: {
        Square sq(randcolor(), randsize());
        c = &sq;
        break;
      }
    }

    list[j] = c;
    j++;  
  }
  return 0;
}

Please help me Expected output should be like: 2 Area constructor is called.. 1 Area constructor is called 0 Area constructor is called

So it should be like: 20 times randomnumber between 0 and 3 "Area constructor is called..."

But it is giving the same number after the second try... in while loop

Answer 1

Not knowing how much depth you want out of this, the problem is you're pushing invalid addresses of temporary objects into your list. as each case is entered, the resulting object is created, addressed, then destroyed. the address will likely be reused on the next loop, but is invalid as soon as the scope for the last object is left.

Try this:

int main()
{
    srand ( time(NULL) );

    Area *list[20];

    int j=0;
    while(j < sizeof(list)/sizeof(*list)))
    {
        switch ( rand() % 4 )
        {
            case 0:
            {
                list[j++] = new Circle(randcolor(),randsize());
                break;
            }

            case 1:
            {
                list[j++] = new Ring(randcolor(),randsize(),randsize());
                break;
            }

            case 2:
            {
                list[j++] = new Rectangle(randcolor(),randsize(),randsize());
                break;
            }

            case 3:
            {
                list[j++] = new Square(randcolor(),randsize());
                break;
            }
        }
    }

    // TODO: Use your Area array list


    // cleanup
    for (int i=0; i<j; ++i)
        delete list[i];

    return 0;
}

I suggest spending some time learning about dynamic allocation. Once you do that, spend even more time learning about the standard library containers and smart pointers that can alleviate you of much of this headache.

---

Alternative (sooner or later you'll want to learn this stuff)

#include <iostream>
#include <vector>
#include <memory>
#include <random>

// include your Area and other declarations here

int main()
{
    std::random_device rd;
    std::mt19937 rng(rd());
    std::uniform_int_distribution<> dist(0,3);


    std::vector< std::unique_ptr<Area> > list;

    for (int i=0; i<20; ++i)
    {
        switch ( dist(rng) )
        {
            case 0:
                list.emplace_back(new Circle(randcolor(),randsize()));
                break;

            case 1:
                list.emplace_back(new Ring(randcolor(),randsize(),randsize()));
                break;

            case 2:
                list.emplace_back(ew Rectangle(randcolor(),randsize(),randsize()));
                break;

            case 3:
                list.emplace_back(new Square(randcolor(),randsize()));
                break;
        }
    }

    // TODO: Use your Area array list
    for (auto& ptr : list)
    {
        ptr->Method();
    }

    return 0;
}

Answer 2

One issue with your code is that you seem to construct temporary objects in the switch cases, and once the scope is over, your c variable will not point to a valid object anymore due to the automated destruction at the end of the case scopes.

The fix would be to either move out the constructions before the switch statement, but still within the while loop, or just use pointers (maybe even smart if you wish).

I will give you an example for one switch case which could be followed for all the rest:

 case 0:
    {
        list[j++] = new Circle(randcolor(), randsize());
        break;
    }