C ++ - Problem saving values - values change after saving

Question

I've been trying for a few days to solve a memory problem.

I have a program where the user enters a vector and its name.

The program stores everything in the vector class.

The problem is that when I print the vector, the values change to other values.

I mean an array whose name is "array" in Vector::Vector

Does anyone know where the problem might be?

Main.cpp:

# include "Midgam.h"

int main(int argc, char **argv) {
    Midgam m(3,20),m1();
    m.Start();
    m.~Midgam();
    return 0;
}

Midgam.cpp:

#include "Midgam.h"

Midgam::Midgam(int num_of_boxes, int num_of_parties):boxNum(num_of_boxes),maxParties(num_of_parties) {
    iterator = 0;
    midgam = new Vector[maxParties];
}

Midgam::~Midgam() {
    if(midgam)
        for(int i=0; i < iterator; i++)
            midgam[i].~Vector();
    delete [] midgam;
}

void Midgam::Start(){
    int choice;
    while (choice != 6){
        cout << "Please enter your choice" << endl;
        cin >> choice;
        switch (choice) {
        case 1:
            AddParty();
            break;
        case 2:
            cout << "case 2" << endl;
            break;
        case 3:
            cout << "case 3" << endl;
            break;
        case 4:
            cout << "case 4" << endl;
            break;
        case 5:
            cout << "case 5" << endl;
            break;
        case 6:
            break;
        default :
            cerr << "ERROR: invalid command; type 6 for exit" << endl;
        }
    }
    cout << "END OF PROGRAM" << endl;
}

void Midgam::AddParty(){
    string name;
    cout << "Print All:" << endl;
    PrintAll();
    cout << "Insert name:" << endl;
    cin >> name;
    if (name.size() > 12){
        cerr << "Party name is too long, please don't pass the 12 characters!!!" << endl;
        return;
    }
    cout << "Insert values:" << endl;
    Vector v(name,boxNum);
    if (!v.getBool()){
        cerr << "ERROR: invalid input" << endl;
        return;
    }
    int exist = FindPartyByName(name);
    cout << "exist: " << exist << endl;
    if(exist != -1){
        cout << "EXIST" << endl;
        midgam[exist]=v;
        return;
    }
    cout << "iterator: " << iterator <<endl;
    if(iterator >= maxParties){
        cerr << "ERROR: no more new parties" << endl;
        return;
    }
    cout << "iteraator: " << iterator << endl;
    midgam[iterator]=v;
    iterator++;
}

int Midgam::FindPartyByName(string party){
    for (int i=0; i < iterator; i++){
        if (party.compare(midgam[i].getName()) == 0){
            return i;
        }
    }
    return -1;
}

void Midgam::PrintAll(){
    for(int i=0; i < iterator; i++){
        cout << midgam[i].getName() << endl;
        midgam[i].PrintArray();
    }
}

Midgam.h:

#include <iostream>
#include <string>
#include "Vector.h"

using namespace std;

#ifndef MIDGAM_H_
#define MIDGAM_H_

class Midgam {
private:
    int boxNum;
    int maxParties;
    int iterator;
    Vector *midgam;
public:
    Midgam(int num_of_boxes, int num_of_parties);
    virtual ~Midgam();
    void Start();
    void AddParty();
    int FindPartyByName(string party);
    void PrintAll();
};

#endif /* MIDGAM_H_ */

Vector.cpp:

#include "Vector.h"

Vector::Vector(string name,int size):name(name),size(size){
    array = new unsigned int[size];
    string results;
    cin.ignore();
    getline(cin,results);
    Bool = StringToArray(results);
}

Vector::Vector(const Vector &v2){
    name=v2.name;
    size=v2.size;
    Bool=v2.Bool;
    array = new unsigned int[size];
    for(int i=0; i < size; i++)
        array[i]=v2.array[i];
}

Vector::Vector(){
    name="";
    size=0;
    array=NULL;
    Bool=true;
}

Vector::~Vector() {
    if(array)
        delete [] array;
}

bool Vector::StringToArray(string str){
    stringstream ss(str);
    for(int i=0; i < size ; i++){
        if(ss.eof())
            return false;
        ss >> array[i];
        cout << array[i];
    }
    if(!ss.eof())
        return false;
    return true;
}

bool Vector::getBool(){
    return Bool;
}

string Vector::getName(){
    return name;
}

void Vector::PrintArray(){
    for(int i=0; i < size; i++)
        cout << array[i] << " ";
    cout << endl;
}

Vector.h:

#include <iostream>
#include <string>
#include <sstream>

using namespace std;

#ifndef VECTOR_H_
#define VECTOR_H_

class Vector {
private:
    string name;
    int size;
    unsigned int *array;
    bool Bool;
public:
    Vector(string name,int size);
    Vector();
    Vector(const Vector &v2);
    virtual ~Vector();
    bool StringToArray(string str);
    bool getBool();
    string getName();
    void PrintArray();
};

#endif /* VECTOR_H_ */

Delete this `m.~Midgam();`. You don't need to call a destructor, it happens automatically, that's the whole point of destructors. — john, Apr 08 '19 at 21:35
It looks like you posted a whole lot of code that has nothing to do with your problem. Please make a [mcve]. Also, [why `eof()` in a loop condition is wrong](https://stackoverflow.com/q/5605125/9254539). — eesiraed, Apr 08 '19 at 21:40
There are probably lots of bugs in this code, but the immediate reason for your error is that you have not defined an assignment operator for your `Vector` class, but you use a `Vector` assignment here. `midgam[iterator]=v;`. You should read up on the [rule of three](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three/4172724). It's your failure to follow that which is the fundamental problem. — john, Apr 08 '19 at 21:44
Of course this code becomes much easier if you avoid pointers, use `std::vector` instead of pointers, and your code is half the size and twice as reliable. — john, Apr 08 '19 at 21:45
It's generally not recommended to make class names that look like STL objects. If you want a vector, you should make it an `std::vector`. — Tzalumen, Apr 08 '19 at 23:17

bruno · Accepted Answer · 2019-04-08T22:08:50.827

As said in remarks you missed to define the operator= on Vector, so the default operator= definition is used and that one copies the content without cloning array.

Because of that when you do midgam[iterator]=v; you share v.array but that one is deleted when v is deleted and later you access to the deleted array in PrintAll with an undefined behavior : in your case that does not print the expected values.

You just have to define the operator=, for instance doing as Vector(const Vector &v2):

Vector & Vector::operator=(const Vector &v2) {
  if(array)
    delete [] array;

  // from Vector(const Vector &v2)`
  name=v2.name;
  size=v2.size;
  Bool=v2.Bool;
  array = new unsigned int[size];
  for(int i=0; i < size; i++)
    array[i]=v2.array[i];

  return *this;
}

After that the execution is :

Please enter your choice
1
Print All:
Insert name:
aze
Insert values:
1 2 3
123exist: -1
iterator: 0
iteraator: 0
Please enter your choice
1
Print All:
aze
1 2 3 
Insert name:
qsd
Insert values:
11 22 33
112233exist: -1
iterator: 1
iteraator: 1
Please enter your choice
1
Print All:
aze
1 2 3 
qsd
11 22 33 
Insert name:
^C

so PrintAll produces the expected outputs

TNX... I get error: ..\Vector.h:20:2: error: extra qualification 'Vector::' on member 'operator=' [-fpermissive] Vector::operator=(const Vector &v2); — Oz1988, Apr 09 '19 at 12:56
@Oz1988 this is because you put the definition directly in the class definition rather than outside. Declare the operator like you declare the other members, so into the class definition `Vector & operator=(const Vector &);` and like others put the definition of the operator outside the class definition — bruno, Apr 09 '19 at 12:59

C ++ - Problem saving values - values change after saving

1 Answers1