-4

I am wondering what is the difference between the following ways to create object of class. The head file is:

class NumberRange {
public:
    NumberRange(int a, int b);
    virtual ~NumberRange();
    void Print(int a, int b);
private:
    int *range_;
    int size;
};

The .cc file is:

#include <iostream>
#include "numberrange.h"
using namespace std;

NumberRange::NumberRange(int a, int b) {
    if (a > b) {
        cout << "a must be equal or less than b" << endl;
    }
}

NumberRange::~NumberRange() {

}

void NumberRange::Print(int a, int b) {
    this->size = b - a + 1;
    this->range_[0] = a;
    for (int i = 0; i < this->size; i++) {
        this->range_[i] = a + i;
        cout << this->range_[i] << endl;
    }
}

int main() {
    NumberRange * numberrange = new NumberRange(5, 9);
    numberrange->Print(5, 9);
}

When I create an object using pointer and compile the program. I got an error says [1] 20346 segmentation fault ./numberrange

but if I change the main function as:

int main() {
     NumberRange numberrange(5,9);
     numberrange.Print(5,9);
}

This would be work. So I don't know when should I use pointer to create an object. Thank you!

Xiufen Xu
  • 531
  • 1
  • 3
  • 19
  • One of the fields of the object is `int *range_`. You are never initializing/allocating this in the constructor, so using it in the Print function will be undefined behavior. Sometimes it will work, sometimes not. Fix that first, and then try again. – Ricky Mutschlechner Oct 25 '16 at 16:44
  • 3
    You missed to allocate `range_`, that simple. So you're calling undefined behavior. Undefined behavior is ... well ... undefined. – πάντα ῥεῖ Oct 25 '16 at 16:44
  • If the second one works, it's only because you get lucky. You access the `range_` object without giving any storage to it, which is undefined behavior. – Chad Oct 25 '16 at 16:45
  • Don't use `new` unless you have a reason. Actually, in modern C++ you should rarely use `new` at all. – aschepler Oct 25 '16 at 16:46
  • BTW, I've seen you struggling with [that stuff](http://stackoverflow.com/questions/40234426/error-when-accessing-members-of-an-instance-of-a-class) recently already. You should better refer to a [good book](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) for learning the c++ language, instead of asking questions at Stack Overflow. – πάντα ῥεῖ Oct 25 '16 at 16:49
  • @aschepler Not exactly, since I saw a lot of examples in cplusplus.com. They use `new ` to create objects a lot. – Xiufen Xu Oct 25 '16 at 17:07
  • @πάνταῥεῖ Yeah, you are right. I will look for some good books. Thank you – Xiufen Xu Oct 25 '16 at 17:09
  • @Chad So can I say that if I create object using `NumberRange numerrange(5,9)`, the numberrange will be stored in stack, and using `new` to create numberrange, it will be stored in memory? – Xiufen Xu Oct 25 '16 at 17:11
  • Without dynamic allocation (using `new`) objects are stored with automatic storage duration. However, that question is not really relevant to the issue you're having which is writing outside the bounds of an array. – Chad Oct 26 '16 at 15:09

2 Answers2

3

You have a pointer range_ in your class, that you do not initialize in constructor. Then you use it in Print method which leads to UB in both cases. The fact, that program does not crash in second case does not mean your program is correct.

Slava
  • 43,454
  • 1
  • 47
  • 90
1

There are at least three problems with your program.

The first, most obvious problem is that int *range_ is a pointer to integer, but doesn't point to anything in particular. It just contains a random memory address. To change that, assign array of int to it, that you created with new, e.g. :

range_ = new int [b - a];

The second, less obvious problem is that you pass a and b two times to your object, first in the constructor and later in the Print method (better not capitalize methods, by the way). One of the things the ++ in C++ offers you is object orientation. Objects have state. If you want objects of class NumberRange to store numbers from 3 upto 7, use your constructor to store them as attributes. No need to pass that boundaries again in the Print method, it can read them from the attributes.

The third, even less obvious problem, is that in your example there's no need to store the numbers at all, since you just want to print them.

Problem two and three are connected. Probably you want to do more with your numbers than just print them. So have the constructor either store a and b, or allocate an array and store the numbers from a upto b. The latter is less efficient, but maybe later on you want to store sequences which are not adjacent integers.

Jacques de Hooge
  • 6,750
  • 2
  • 28
  • 45