0 
#include <iostream>
#include <string>
using namespace std;

class Part{
public:
    std::string spec;
    Part(std::string str){
        this->spec = str;
    }
    std::string getSpec(){
        return spec;
    }
};

class Car{
public:
    Part getEngine();
    Part getWheels();
    Part getBody();
};


class Benz:public Car{
public:
    Part getEngine(){
        return Part("Benz Engine");
    }   
    Part getWheels(){
        return Part("Benz Wheels");
    }
    Part getBody(){
        return Part("Benz Body");
    }
};

class Audi:public Car{
public:
    Part getEngine(){
        return Part("Audi Engine");
    }   
    Part getWheels(){
        return Part("Audi Wheels");
    }
    Part getBody(){
        return Part("Audi Body");
    }
};

class CarFactory{
public:
    Car *pcar;
    Car *getCar(std::string carType){
        if (carType.compare("Benz") == 0){
            pcar = new Benz();
        }else if (carType.compare("Audi") == 0){
            pcar = new Audi();
        }
        return pcar;
    }
};

int main(){
    CarFactory *cf = new CarFactory();
    Car *mycar = cf->getCar("Benz");
    Part myCarBody = mycar->getBody();
    cout <<myCarBody.getSpec() <<endl;
    //cout << mycar->getBody().getSpec() <<endl;
}

In the above code getting, undefined reference to object error at line Part myCarBody = mycar->getBody(); line of main function. Can you please help me for fixing this and also please explain the difference between Car *mycar and Car mycar. Which one to choose when?

Thanks for the help.

Improve this question
3
  • Car *mycar is a pointer to a Car; Car mycar is an actual Car object. You need to use the arrow (->) notation with the pointer; you need to use the dot (.) notation with the object. In this context, a pointer is more likely to be beneficial than an object. It is often that way, but not always. Commented May 1, 2015 at 7:08
  • The inheritance model in this code leaves much to be desired. All Benz objects behave the same; all Audi objects behave the same. That's bad. It's antique, but Tom Cargill's book C++ Programming Style goes through this and some other common problems in some detail. Commented May 1, 2015 at 7:12
  • @Kranthi Please accept either answer. It's about time. Commented Sep 1, 2015 at 21:14

2 Answers 2

Reset to default
6

The member functions of Car should be pure virtual as opposed to your non-virtual declarations:

class Car
{
public:
    virtual ~Car() = default;     // Virtual destructor
    virtual Part getEngine() = 0;
    virtual Part getWheels() = 0;
    virtual Part getBody() = 0;
};

That's why the linker error. Compiler expects Car's function definitions because the functions aren't pure virtual, but it couldn't find it.
With pure virtual member functions, Car becomes an abstract class and provides an interface (pure virtual), that must be implemented by classes derived from it. Otherwise, the derived class will be abstract too and not instantiable.

By calling a virtual function of a base class, you end up calling a function of a derived class that overrides it. Destructor is also the case and needs to be made virtual, in order to call the destructor of the derived class and avoid undefined behavior. If you had a dynamically allocated data in one of the derived classes, there would be a memory leakage.

Also, you have to use a pointer or a reference when dealing with polymorphism. The logic behind it is that pointers have the same size, but a derived class like Benz could have additional data and not fit in the space occupied by Car and would become sliced. Thus, assignment/copy construction do not work properly without a pointer or reference.

Suggested in comments:

  • Accessor functions ("getters" and "setters") come usually with private or protected members. Yours are public, the accessor is useless and can be circumvented. Keep the accessor, make the data private. When you're returning a member, you can do it by reference-to-const. But don't do it for getEngine, getWheels and getBody, because they're returning a local object, not a member.

    std::string const& getSpec() { return spec; }

    While we're at it, you can pass std::string by reference-to-const too.

    Part(std::string const& str) : spec(str) {}

  • Factory class

    class CarFactory
{
public:
    Car *getCar(std::string const& carType)
    {
        if(carType == "Benz") // std::string::compare is for comparing by alphabetical order
            return new Benz();

        if(carType == "Audi")
            return new Audi();

        return nullptr; // invalid carType, return nullptr and check for that in main()
        // You were returning previously constructed Car, was that desired?
    }
};

You're also forgetting to delete myCar, see more good info in the other answer.

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

Correct. But there are some other bad stuff in the code. I see evrywhere public data members which is not a godd idea. And in the factory class a nullptr shall be returned if the given is string is not Benz nor Audi.
@vizhanyolajos Not only that problems - also object copying, possibility of creation Part implicitly from std::string, memory leak...
5

First if you want to use polymorphism you need to declare functions as virtual.

If you don't plan to implement methods in base class you may declare them as pure virtual. This make your base class so called "abstract" and you will can't create objects of that class (in your example best candidate is Car object).

Also beware. If you want to destroy objects via pointer to base class you also need virtual destructor.

So to put all together you need:

class Car {
public:
    virtual ~Car() {}
    virtual Part getEngine() = 0;
    virtual Part getWheels() = 0;
    virtual Part getBody() = 0;
};

So the root cause of your problem that you said to compiler "Hey, I'm going to implement methods of Car" but hadn't done it. So compiler expected to call that methods from your main but on linking stage linker can't find them.

Another problems you need to think about:

  1. Who will destroy objects you have created by "new". I suggest not to think about it and use shared (smart) pointers (see std::shared_pointer container).
  2. When you declare constructor of Part class from string you declare possibility of creation Parts from string as Part x = "Part Name"; If you don't like this behaviour read about "explicit" keyword.
  3. As was said in comments for bigger project you need to think about encapsulation an hide details of object implementation in "private" sections of classes.
Improve this answer

5 Comments

Hi well explained. Can you show me the sample code how to use smart pointers in my code.
Just replace Car * to std::shared_ptr<Car>
And also when you creates object use std::shared_ptr<Car>(new Benz)
Tried std::shared_ptr<CarFactory> cf = new CarFactory();. getting "'cf' was not declared in this scope" and "'shared_ptr' is not a member of 'std'" error. Include <memory> header file.
This is part of C++-11 standard. Check how to enable it for your compiler.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.