c++ assignment operator only working on objects already initialized

Question

Hi my issue (at least how it seems) is that I cannot use the overloaded assignment operator when declaring the variable. I can only use it when the object already has some values in it.

In my testing file I have the following lines of code which does not use the assignment operator as wanted, and will crash the program.

JumblePuzzle jp2("foo", "easy");
JumblePuzzle jp1 = jp2;
cout << "Attempted assignment" << endl;

However, the following lines will work.

JumblePuzzle jp2("foo", "easy");
JumblePuzzle jp1("bar", "easy");
jp1 = jp2;
cout << "Attempted assignment" << endl;

The JumblePuzzle Class

class JumblePuzzle{
public:
    JumblePuzzle(string word, string difficultyString);
    ~JumblePuzzle();
    JumblePuzzle(const JumblePuzzle &puzzle);


    charArrayPtr* getJumble() const;
    charArrayPtr* Jumble() const;
    int getSize() const;
    int getRowPos() const;
    int getColPos() const;
    char getDirection() const;
    bool placeWord();
    string getWord() const;

    JumblePuzzle& operator=(const JumblePuzzle &puzzle); //relevant

private:
    int size;
    char direction;
    string word;
    int rowPos;
    int colPos;
    char** jumble;
};

The JumblePuzzle Constructor, the only part where I have had other issues is with the allocation of memory to jumble.

JumblePuzzle::JumblePuzzle(string word, string difficultyString){
    if(!difficultyString.compare("easy"))
        size = word.length() * 2;

    else if(!difficultyString.compare("medium"))
        size = word.length() * 3;

    else if(!difficultyString.compare("hard"))
        size = word.length() * 4;


    this->word = word;
    rowPos = 0;
    colPos = 0;
    direction = '\0';


    jumble = (char**) malloc(size*sizeof(char *));
    for(int i = 0; i < size; i ++)
        jumble[i] =(char*) malloc(size*sizeof(char));


    for(int i = 0; i < size; i ++){
        for(int j = 0; j < size; j ++){
            jumble[i][j] = '\0';
        }
    }
    while(!this->placeWord());
}

JumblePuzzle copy constructor

JumblePuzzle::JumblePuzzle(const JumblePuzzle &puzzle){
    size = puzzle.getSize();
    rowPos = puzzle.getRowPos();
    colPos = puzzle.getColPos();
    direction = puzzle.getDirection();
    word = puzzle.getWord();

    const charArrayPtr *tempJumble = puzzle.Jumble();
    jumble = (charArrayPtr*) malloc(size*size*sizeof(char));

    memcpy(jumble, tempJumble, size*size);
}

The overloaded assignment operator

JumblePuzzle& JumblePuzzle::operator=(const JumblePuzzle &puzzle){

        cout << "in assignment operator" << endl;
        size = puzzle.size;
        direction = puzzle.direction;
        rowPos = puzzle.rowPos;
        colPos = puzzle.colPos;
        word = puzzle.word;


        jumble = (charArrayPtr*) malloc(size*sizeof(char *));
        for(int i = 0; i < size; i ++)
            jumble[i] = (char*) malloc(size*sizeof(char));
        jumble = puzzle.jumble;

        return *this;

}

This code is for a school assignment and my prof doesn't want us to change the testing file code, which is why I need the first segment of code to work.

I have tried searching for solutions for quite a while, and would really appreciate if someone could provide some insight.

Answer 1

JumblePuzzle jp2("foo", "easy");
JumblePuzzle jp1 = jp2;
cout << "Attempted assignment" << endl;

crashes the program because it's using the default copy constructor, which directly copies the jumble pointer from jp2 to jp1; they end up with the same pointer, and the code crashes when both destructors try to free the same memory.

By contrast,

JumblePuzzle jp2("foo", "easy");
JumblePuzzle jp1("bar", "easy");
jp1 = jp2;
cout << "Attempted assignment" << endl;

"works", because you hand-implemented operator= to allocate new memory. It's still not correct though, because the constructor for jp1 already allocated memory, which you neither free nor reuse, you just overwrite the pointers and create a memory leak.

For a fix, I'd strongly recommend getting away from using raw pointers. jumble could presumably be stored as a std::vector<std::string> which would remove the need for manual memory management entirely, and make the default copy constructor behave correctly automatically. If you're required to manage the memory yourself, you're going to need to hand-write a copy constructor, and make sure both the copy constructor and the copy assignment operator (operator=) are properly allocating (and in the case of assignment, deallocating) memory. A helpful starting point is to realize you don't actually need to implement both manually, if you use the copy-and-swap idiom.