How to declare a <vector> object and use push_back inside a class?

Question

I'm trying to build a class named "Tombola" which should contain as private variable an empty vector. This should be filled at runtime through the class member Tombola.estrai(), which generates a random number and insert it inside the vector named "order" by the method order.push_back(number). This is the class definition in the tombola.h header:

#ifndef TOMBOLA_H
#define TOMBOLA_H
#include <cstdlib>
#include <vector>
using namespace std;
class Tombola {
 private:
 bool on_off[90];
 int tabellone[9][10];
 int x_max = 9;
 int y_max = 10;
 vector<int> order;
 public:
 Tombola();
 ~Tombola();
 void nuovo();
 int estrai();
 bool completato();
 void stampa();
 void stampa_tab();
};
#endif

And this is the implementation of constructor/destructor and Tombola::estrai() inside tombola.cc:

#include "tombola.h"
#include <cstdlib>
#include <cmath>
#include <ctime>
#include <vector>
#include <iostream>
using namespace std;
Tombola::Tombola () {
 vector<int> ord;
 order = ord; 
 int z=1;
 for(int i=0;i<90;i++) {
 on_off[i] = false;
 }
 for(int j=0;j<=x_max;j++) {
 for (int k=0;k<=y_max;k++) {
 tabellone[j][k] = z;
 z++;
 }
 }
}
Tombola::~Tombola() {
 cout << "Tombola is destroyed" << endl;
}
int Tombola::estrai() {
 srand(time(NULL));
 int estrazione = int(ceil(rand()/double(RAND_MAX)*90));
 on_off[estrazione]==true;
 order.push_back(estrazione);
 return order.back();
}

and this is the call to the method in the main.cpp file:

#include "tombola.h"
#include <cstdlib>
#include <iostream>
#include <vector>
#include <ctime>
using namespace std;
int main () {
 Tombola natale;
 cout << natale.estrai();
}

When I compile the program everything goes fine, but when I execute the main I get a segmentation fault error which seems to be due to some sort of allocation error when trying to store the item inside the order vector, as reported by the debugger. Could someone explain to me how to solve the error and why the error occours? Thank you.

• Adrian Mole: Change your for(int j=0;j<=x_max;j++) to for(int j=0;j<x_max;j++) to avoid out-of-bounds access to the arrays (and same for the inner loop).
• Retired Ninja: These two lines in the constructor are unnecessary: vector<int> ord; order = ord;
• Botond Horváth: on_off[estrazione]==true; just checks if the 2 things equals. If you want to set it true use = not ==
• Peter: Also, using namespace std in a header file is a bad idea, as it opens the door for a range of significant problems in all source files (e.g. broken build due to ambiguity) that #include it. Although it involves more typing, use fully qualified names (e.g. std::vector) in header files.

Answer 1

The reason of segmentation fault is in the constructor. You have to change for(int j=0;j<=x_max;j++) to for(int j=0;j<x_max;j++) in order not to cross the bounds of the array.

for(int j=0;j<x_max;j++) {
 for (int k=0;k<y_max;k++) {
 tabellone[j][k] = z;
 z++;
 }
} 

However, there are also some minor issues in the code that are worth being mentioned

• declaring default-initialized ord vector and assigning it to order is pointless because order is already default-initialized.(See member initializer list for more information).
• using namespace std; in a header file is a terrible idea, because if you had a large codebase, and had multiple source files where you want to include that header, everywhere the using statement will be applied, which probably is not desired.

• Vittorio: Thank you, I was sure the error was due to the vector declaration that I forgot to check the rest. Also you've been clear about the minor issues.