CODEWITHSUNDEEP
c++char
Gabby Moore
Gabby Moore

Reputation: 263

Getting weird return value from char* in c++

I'm new to c++ programming and i'm getting a weird value coming back from a char* variable that i've set, depending on how i use it. I'm obviously doing something really stupid but I can't see the problem. The next couple of paragraphs describe the setup(badly), but it's probably easier to just look at the output and code.

Basically, I have a couple of classes - Menu and MenuItem. The MenuItem class has a name which is of type char*. Depending on how i'm using the menu items, i'm getting strange results when i do a getName() on the MenuItems.

I have a Machine class which has a state (TestState). This TestState creates a Menu containing MenuItems. When i create a TestState in my main function and have it print out the menu I get what I expect. When i create a Machine which contains a TestState and ask it to print the menu it prints something weird for the name of the root item in the menu.

Output - Last line I'm expecting menuItem1, but i get Hâ∆Hã=ò

Output direct from TestState Object

Displaying menu state 
menuItem1
root not null 
menuItem1


Output from TestState within Machine

Displaying menu state 
menuItem1
root not null 
Hâ∆Hã=ò

Here's my code - Main.cpp

#include "Menu.h"
#include "Machine.h"
#include <iostream>

using namespace std;

Machine m;
TestState t;

int main(void) {
    cout << "Output direct from TestState Object" << endl << endl;
    t = TestState();
    t.print();


    cout << endl << endl << "Output from TestState within Machine" << endl << endl;
    m = Machine();
    m.printCurrentState();
}

Menu.h

#ifndef Menu_h
#define Menu_h

#include <stdlib.h>

class MenuItem {
public:
    MenuItem();
    MenuItem(const char* itemName);
    const char* getName() const ;

protected:
    MenuItem *next;
    const char* name;
};

class Menu {
public:
    Menu(MenuItem *rootItem);
    Menu();
    void setRoot(MenuItem *r);
    MenuItem* getRoot() ;
protected:
    MenuItem *root;
};

#endif

Machine.h

#ifndef MACHINE_H_
#define MACHINE_H_

#include "Menu.h"

class TestState;
class Machine;

class TestState {
public:
    TestState();
    virtual ~TestState();
    void print();
protected:
    Machine* machine;
    MenuItem menuItem1;
    Menu menuMain;
};

class Machine {
public:
    Machine();
    void printCurrentState();
protected:
    TestState testState;
};

#endif /* MACHINE_H_ */

Machine.cpp

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

TestState::TestState() {
    menuItem1 = MenuItem("menuItem1");
    menuMain = Menu(&menuItem1);
}

void TestState::print(){
    cout << "Displaying menu state " << endl;
    cout << menuItem1.getName() << endl;

    if (menuMain.getRoot() == NULL) {
        cout << "root is null" << endl;
    } else {
        cout << "root not null " << endl;
        cout << menuMain.getRoot()->getName() << endl;
    }
}

TestState::~TestState() {
    // TODO Auto-generated destructor stub
}

Machine::Machine() {
    testState = TestState();
}

void Machine::printCurrentState() {
    testState.print();
}

Any help would be appreciated. I'm a bit lost. Thanks Dave

Upvotes: 0

Views: 385

Answers (4)

user34537
user34537

Reputation:

Instead of writing Thing name = Thing(ctorParams); make name a pointer and use new Thing(ctorParams);. It looks like you thought you were using pointers but it worked without the new keyword so you went ahead and not used them which caused you bugs.

Upvotes: 1

Oktalist
Oktalist

Reputation: 14714

TestState::TestState() {
    menuItem1 = MenuItem("menuItem1");
    menuItem2 = MenuItem("menuItem2");
    menuMain = Menu(&menuItem1);
    menuMain.add(&menuItem2);
}

Machine::Machine() {
    testState = TestState();
}

The Machine constructor constructs a temporary TestState and copies its data members into Machine::testState. When the Machine constructor is done, the temporary TestState disappears, but Machine::testState.menuMain.root still points to a member of the temporary.

How to fix:

Learn what is meant by each of the various ways that exist to initialize variables, and how to use initialization lists in your constructors.

Upvotes: 1

Wug
Wug

Reputation: 13196

I suspect what's happening is Menu.root is pointing to a temporary object somewhere. You'll notice that you make a copy of your machine in your main function:

// in main():
m = Machine(); // makes a machine, then copies it

That machine has a TestState, which has a MainMenu, which has a pointer to a MenuItem:

// in MenuItem class definition:
MenuItem *root;

That pointer gets initialized to the address of a member of your original Machine. Problem is, that object only exists for a short time: it's destroyed when the copy is complete, leaving you with a dangling pointer.

In other words, you need to make sure that when you copy an object that contains pointers, you update those pointers to reflect the address of the duplicated object instead of the old one.

You need to add copy constructors like the following:

Machine::Machine(const Machine& other)
{
    teststate = other.teststate;
    teststate.machine = this; // you will need to expose TestState.machine to Machine
}

TestState::TestState(const TestState& other)
{
    machine = other.machine; // Machine copy constructor modifies this for us

    menuItem1 = other.menuItem1; // these 3 we have to do
    menuItem2 = other.menuItem2;
    menuMain = other.menuMain;

    menuMain.setRoot(&menuItem1); // update pointers to be to persistent copies
    menuItem1.setNext(&menuItem2);
    menuItem2.setNext(NULL);
}

You might notice that your system is rather brittle. I'd recommend relying on pointers between objects less, because dragons be down that road.

Upvotes: 4

ecatmur
ecatmur

Reputation: 157324

Machine doesn't have a copy constructor, so the various pointers and references within Machine (specifically, within TestState) are pointing to garbage.

Upvotes: 0

Related Questions