ale
ale

Reputation: 11830

Pointers, polymorphism and segmentation fault in C++

I have a Parent class with a number of children. Each function in the Parent class is pure i.e. there are no parent implementations of the functions but the children have their own implementations. No need to post code there - standard stuff.

I don't want people creating direct instantiations of the parent class from anywhere. I have safe-guarded against this by having the virtual functions being pure so that's fine.

My problem: Depending on a user's input (a string), I want to instantiate one of the children. I only know which one at runtime. My (incorrect?) idea was the following which compiles fine and works fine until I put the code into a function and return the parent.

So this works:

Parent* parent;
if(user_input == "A") {
    Child1 child1;
    parent = &child1;
}
else if(user_input == "B") {
    Child2 child2;
    parent = &child2;
}

but this does not work:

Parent* foo(string user_input) {
    Parent* parent;
    if(user_input == "A") {
        Child1 child1;
        parent = &child1;
    }
    else if(user_input == "B") {
        Child2 child2;
        parent = &child2;
    }
   return parent;
}

When I say it doesn't work I mean, it compiles but then I get a segmentation error when I do this:

Parent* parent = foo(user_input);
parent->some_child_function(); // segmentation error here

I'm sure it's a stupid/simple question about me not fully understanding pointers??? Well after reading all about them in books/articles, I still don't know what I'm doing wrong.... It's probably a one-line fix?

Thank you :).

Upvotes: 4

Views: 1104

Answers (6)

grocky
grocky

Reputation: 573

You should look into the Factory pattern for this solution. Using this pattern will allow any users creating different parent objects to only worry about the factory's interface. And it will also allow you to add different types more easily down the road. Using this pattern may seem like it's an additional extra step, but it adds a nice layer of abstraction to your project and a little more flexibility for adding things later on.

class ParentFactory
{
    public:
          Parent* foo(const std::string &user_input) 
          {
              if(user_input == "A")
                  return new Child1;
              if(user_input == "B")
                  return new Child2;
              return 0;
          }
};

Upvotes: 1

Michael Anderson
Michael Anderson

Reputation: 73580

Slightly shorter version of some of the other examples... get rid of the Parent pointer altogether, and allocate your variable using new. Don't forget to destroy it later using delete.

Parent* foo(string user_input)
{
    if(user_input == "A") {
        return new Child1();
    }
    else if(user_input == "B") {
        return new Child2();
    }
   return NULL;
}

Upvotes: 4

Charbel
Charbel

Reputation: 14697

try this

Parent* foo(string user_input) {
    Parent* parent;
    if(user_input == "A") {
        Child1 *child1 = new Child1();
        parent = child1;
    }
    else if(user_input == "B") {
        Child2 *child2 = new Child2();
        parent = child2;
    }
   return parent;
}

Basically in your version the object was created on the stack, and by the time your method returned it gets purged. using new ensures the object is created on the heap, so it remains there until deleted.

But callers of your method need to make sure they delete the objects otherwise you have memory leaks.

For production code using some library like boost is more appropriate.

Upvotes: 5

Nikolai Fetissov
Nikolai Fetissov

Reputation: 84239

You have to allocate instances of the implementation classes on the heap with operator new. You also want to look into Factory Method Pattern.

Upvotes: 3

Grigor Gevorgyan
Grigor Gevorgyan

Reputation: 6853

In your functions parent is a pointer to a local object, which is destroyed as soon as the program leaves the function scope. Try constructing object in heap, like parent = new child1; and don't forget to delete parent after use.

Upvotes: 1

sharptooth
sharptooth

Reputation: 170559

You run into undefined behavior:

Parent* parent;
if(user_input == "A") {
    Child1 child1; // stack-allocated object
    parent = &child1;
} //child1 is destroyed at this point, the pointer is dangling now

you have to use new (preferably with a smart pointer):

Parent* parent = 0;
if(user_input == "A") {
   parent = new Child1();
} else if(user_input == "B") {
   parent = new Child2();
}

and it's also a good idea to set the pointer to null at the beginning so that you don't have a pointer with a garbage value in case you have an error in your code and the pointer is left uninitialized.

Upvotes: 11

Related Questions