1
\$\begingroup\$

I want to choose the implementation of an abstract class based on settings from a config file. Is this the proper way of doing that?

I am asking because I am having doubts about using std::make_unique inside the if scope. Will the object created with std::make_unique still be valid outside of the if scope? 

#include <iostream>
#include <memory>

// Base class for all position systems
class PositionSystem {
public:
    virtual int getX() const = 0;
};

// GPS implementation
class GPS: public PositionSystem {
public:
    int getX() const {return 42;};
};

// Galileo implementation
class Galileo: public PositionSystem {
public:
    int getX() const {return 21;};
};

int main(){
    // Settings are read from config file
    // and stored in a smiliar struct
    struct {std::string position_system = "Galileo";} settings;

    // Declare position system    
    std::unique_ptr<PositionSystem> ps;

    // Choose implementation based on settings
    if(settings.position_system == "GPS"){
        ps = std::make_unique<GPS>(GPS());
    }
    else if (settings.position_system == "Galileo"){
        ps = std::make_unique<Galileo>(Galileo());
    }

    // Use interface
    std::cout << ps->getX() << '\n';

    return 0;
}
\$\endgroup\$
2
  • \$\begingroup\$ The scope of a variable is the block in which the variable is declared. In this case ˋps` is declared in ˋmain` so the code would work as expected. \$\endgroup\$ Commented Jul 1, 2017 at 11:39
  • \$\begingroup\$ @Eskilade Instead of accepting the 1st answer hastliy, you should thoroughly read about @Loki's one! \$\endgroup\$ Commented Jul 1, 2017 at 17:55

2 Answers 2

Reset to default
3
\$\begingroup\$

When you have a derived type deleted via a pointer to the base class you need to define a virtual destructor. This also applies when you are using unique_ptr.

// Base class for all position systems
class PositionSystem {
public:
    virtual ~PositionSystem() {}    // You need this or your object will 
                                    // not be correctly destroyed.
    virtual int getX() const = 0;
};

You don't need to create an objet and copy it.

    ps = std::make_unique<GPS>(GPS());
                           //  ^^^^^    This creates a automatic object
                           //           When make_unique calls new
                           //           this automatic object is copy constructed
                           //           into the dynamic object.

Simply do this:

    ps = std::make_unique<GPS>();

You have the correct answer. Its an if/switch/command patter that you use to create the correct object. But you don't need to use a unique pointer. You could use a reference rather than a unique_ptr (but a unique_ptr is simpler).

But rather than do it in main I would move this work into a factory specifically for this job.

std::unique_ptr<PositionSystem> ps = positionSystemFactory(settings);

Then you just need to write the factory method:

std::unique_ptr<PositionSystem> positionSystemFactory(Settings const& settings)
{
    // Choose implementation based on settings
    if(settings.position_system == "GPS"){
        return std::make_unique<GPS>();
    }
    else if (settings.position_system == "Galileo"){
        return std::make_unique<Galileo>();
    }
    throw Failure;
}

am having doubts about using std::make_unique inside the if scope.

Yes it will work as expected.

else if (settings.position_system == "Galileo"){
        ps = std::make_unique<Galileo>(Galileo());
    }

Even though the temp object created by make_unique() will be destroyed when it goes out of scope; this object is assigned to the variable ps where its content is maintained.

How this works: The temp object invokes the templatized move constructor on ps thus resulting in the contained pointer being moved from the temp object into ps (which takes ownership away from the temp object). When the temp object is destroyed it no longer owns the pointer and thus does not delete it.

\$\endgroup\$
6
  • \$\begingroup\$ +1 for mentioning references, I was unsure about that as well. Out of curiosity though, do I really need a virtual destructor when using smart pointers? This post seems to suggest otherwise : Stack Overflow[stackoverflow.com/questions/20802810/… \$\endgroup\$ Commented Jul 1, 2017 at 18:58
  • \$\begingroup\$ @Eskilade The first line of the answer you link: delete a is undefined behaviour, because the class Base does not have a virtual destructor So yes you need a virtual destructor of you are calling delete via a pointer to a base class. Here: std::unique_ptr<PositionSystem> ps; you are holding the pointer via a pointer to the base class. \$\endgroup\$ Commented Jul 2, 2017 at 17:12
  • \$\begingroup\$ You mention : 'You could use a reference rather than a unique_ptr (but a unique_ptr is simpler)' How would you go about using references rather than smart pointer? \$\endgroup\$ Commented Oct 13, 2017 at 17:47
  • \$\begingroup\$ PositionSystem& ps = choosePositionSystem(config); Then in choosePositionSystem() you have bot object as static members and return a reference to one of them. \$\endgroup\$ Commented Oct 13, 2017 at 19:29
  • \$\begingroup\$ Nice, thanks. Is there any particular reason why one would prefer references over smart pointers? My gut is telling me that using references to static variables may cause trouble depending on how the factory function is used, e.g. several calls to factory thus creating several references to same object then using them in a multi-threaded environment \$\endgroup\$ Commented Oct 13, 2017 at 21:00
1
\$\begingroup\$

Yes, it is going to be available - it's going to live as long as the ps wrapper does as it owns the object now.

By the way you may want to consider creating some enum holding the type of positioning system you use instead of comparing strings as I suspect you may want to check against the positiong system type multiple times within your application.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.