-1

I was trying a random code to accept values using dynamic size. Surprisingly the for loop in the Accept function does not execute. Instead, the control directly leaves the function. Please explain what is the mistake here?

using namespace std;
#include<iostream>

class consequtive
{
public : int *ptr;
         int size;

public : 
    consequtive(int size);
    void Accept();
};

consequtive::consequtive(int size)
{
     ptr = new int[size];
}

void consequtive :: Accept()
{

    cout<<"Enter elements :: "<<endl;

    for(int i = 0 ; i < size ; i++)
    {
        cin>>ptr[i];
    }

}

int main()
{
    int size = 0;
    cout<<"Enter size ::";
    cin>>size;

    consequtive obj(size);
    obj.Accept();
}
Oswald Vinny
  • 53
  • 1
  • 10
  • 5
    You never set the `size` member variable in class `consequtive` so its a random garbage value. – drescherjm Jan 28 '18 at 19:10
  • The user gives an input value for size which is passed in the parameterized constructor, which is then used to create an array of mentioned size. Isn't it? – Oswald Vinny Jan 28 '18 at 19:11
  • 3
    That variable has no relation at all to the class member `size` – drescherjm Jan 28 '18 at 19:12
  • 3
    There are 3 different variables all named `size`. – 1201ProgramAlarm Jan 28 '18 at 19:12
  • 2
    It is not set in your class. You pass it as the argument of the constructor, but never initialize or assign it. If you add the (awkward) assignment `this->size = size;` above the `new` statement, your code works. – Chiel Jan 28 '18 at 19:12
  • Furthermore, since your code DOES print "Enter elements", it tells you that the `size` that is used in the for loop, does not have the correct value. Use a debugger. @drescherjm gave you the answer: it is undefined thus random garbage. – Chiel Jan 28 '18 at 19:15
  • 2
    @Chiel I think you mean `this->size = size;`. – cpplearner Jan 28 '18 at 19:16
  • @cpplearner. You are correct. I edited it. – Chiel Jan 28 '18 at 19:16
  • 1
    Slightly off topic, but: I recommend you learn how to use a debugger. You might have been able to figure this one out yourself if you had done so. – Tim Seguine Jan 28 '18 at 19:23

3 Answers3

3

A few problems here.

  1. You have a class parameter that has the same name as a member, which isn't really a problem, but is a source of confusion (As in your case).
  2. You never set the member size to anything inside the constructor.

For number one, I would recommend renaming the class member size to size_ or something similar, since this creates a separation and makes the variables easier to distinguish from each other. As for as the second problem, I would change your constructor to the following:

consequtive::consequtive(int size) : size_(size) // Assuming the member is called `size_`
{
    ptr = new int[size];
}

The code should work now, and uses a concept called member initializer lists. Not setting the variable size results in undefined behavior.

Arnav Borborah
  • 11,357
  • 8
  • 43
  • 88
1

You forgot to initialize the size member variable.

You could do something like this:

consequtive::consequtive(int size)
:   size(size),
    ptr(new int[size])
{
}

You should also add a destructor to your class, to avoid a memory leak:

consequtive::~consequtive()
{
    delete[] ptr;
}
Sid S
  • 6,037
  • 2
  • 18
  • 24
  • Would this work? Which `size` is the one passed into `new int[size]`? Is it the parameter or the member? – Mike Vine Jan 28 '18 at 19:34
  • Yes, it works. The parameter is used when both are present. Same thing with assignment in the constructor body - you would have to qualify the member with `this->` to not use the parameter. – Sid S Jan 28 '18 at 22:30
0

This size in the class definition

public : int *ptr;
         int size;

this size in the constructor implementation

consequtive::consequtive(int size)

and this size in the main function

int size = 0;

are all different variables. The latter two will both have the same value because of the way they are used, but one size can be changed to a different value without the other being aware. The bug in the asker's code is because the first size is never given a value and is used uninitialized.

Solution:

consequtive::consequtive(int size): ptr(new int [size]), size(size)
{
}

Here we are using the Member Initializer List. We don't gain much from its use in this case, but it is a very useful tool. More on that here: Why should I prefer to use member initialization list?

Be cautious when using a parameter or local variable with the same name as a member. The inner most identifier always wins so inside

consequtive::consequtive(int size): ptr(new int [size]), size(size)
{
    // in here
}

the size variable is the parameter and not the member. You can this->size to explicitly state you want the member, but it is a better idea to not reuse the identifier at all. You could forget to prepend this-> and the compiler is unlikely to warn you of the mistake.

user4581301
  • 33,082
  • 7
  • 33
  • 54