Code crashes on setting class variable

Question

I was just writing little OOP app and got crash while running (not compiling) the app on setting class's private string variable through the setter, here's the header file :

class Car
{
private:
int year;
std::string brand;
std::string model;
int price;
std::string currency;
public:
int setYear(int x){this->year = x;}
std::string setBrand(std::string x){this->brand = x;}
std::string setModel(std::string x){this->model = x;}
int setPrice(int x){this->price = x;};
std::string setCurrency(std::string x){this->currency = x;}
};

and here's the main: n - number of objects temp - temporary variable for passing integers temp1 - temporary variable for passing strings

ifstream fd("input.in");
int n;
fd >> n;
int temp;
string temp1;
Car A[n];
for(int i = 0; i < 3; i++)
{
    fd >> temp;
    A[i].setYear(temp);
    fd >> temp1;
    A[i].setBrand(temp1);  //Crashes Here
    fd >> temp1;
    A[i].setModel(temp1);
    fd >> temp;
    A[i].setPrice(temp);
    fd >> temp1;
    A[i].setCurrency(temp1);
}

after little test i figured out that it crashes then code tries to set "brand" variable. What's the problem?

Please review the differences between arrays and vectors, you are breaking a cardinal rule in coding. I would first suggest to use vectors, but if you insist on using arrays then you will need to allocate memory dynamically in this instance. — Andrew Corsini
– Andrew Corsini, Commented Dec 31, 2012 at 14:52

Lightness Races in Orbit · Accepted Answer · 2012-12-31 15:43:57Z

5

Array dimensions must be known at compile-time, so:

C A[n];

is wrong.

GCC supports variable-length arrays as a non-standard extension but, even if you're accidentally using them, your loop assumes n == 3 with no apparent indication that this is necessarily true.

Instead, use a vector:

std::vector<C> A(n);

and iterate over it properly:

std::vector<C>::iterator it = A.begin(), end = A.end();
for ( ; it != end; ++it) {
   // your for loop stuff with *it
}

or, in C++11:

for (auto& a : A) {
   // your for loop stuff with a
}

edited Dec 31, 2012 at 15:43

answered Dec 31, 2012 at 14:50

Lightness Races in Orbit

387k77 gold badges670 silver badges1.1k bronze badges

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

2 Comments

Ryan Guthrie Over a year ago

Since he is setting stuff in the vector, he would need a ref for the for(auto &a : A) example.

Lightness Races in Orbit Over a year ago

@Ryan: True say. BTW I've been to Boulder; nice place.

David G · Accepted Answer · 2012-12-31 16:01:22Z

1

In addition to Lightness's answer, I noticed that the methods of your Car class have return types but no return statement. Runtime errors usually overshadow most compilation errors, so that's probably why it didn't come to your attention. To resolve this, replace the return values of your "set" methods with void, meaning the function doesn't return anything. Do this to all your methods as they all lack return statements.

edited Dec 31, 2012 at 16:01

answered Dec 31, 2012 at 15:10

David G

97.6k41 gold badges173 silver badges258 bronze badges

1 Comment

Lightness Races in Orbit Over a year ago

Good spot about the returns, though.

fatma.ekici · Accepted Answer · 2012-12-31 18:56:34Z

0

How did it not give any compile time errors? Below statement should cause an error since n is not known at compile time. You should either make A as a std::vector or use a macro definition or static const for "n".

    Car A[n];

Moreover you do not need any return value for setter functions. They do not return anything although the function signatures show that they should.

answered Dec 31, 2012 at 18:56

fatma.ekici

2,8375 gold badges32 silver badges31 bronze badges

1 Comment

Lightness Races in Orbit Over a year ago

With silly loose compilation flags GCC will attempt to employ VLAs in some scenarios.

Collectives™ on Stack Overflow

Code crashes on setting class variable

3 Answers 3

2 Comments

1 Comment

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

1 Comment

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related