18

I'm writing a piece of generic software that will be loaded on to many different variants of the same basic hardware. They all have the same processor, but with different peripherals and their own functions that need to be carried out. The software will know which variant it should run by reading a hardware switch value.

Here's my current implementation in a nutshell:

class MyBase
{
public:
    MyBase() { }
    virtual run() = 0;
}


class VariantA : public MyBase
{
public:
    VariantA () { }
    virtual run()
    {
        // Run code specific to hardware Variant-A
    }
}


class VariantB : public MyBase
{
public:
    VariantB () { }
    virtual run()
    {
        // Run code specific to hardware Variant-B
    }
}


void main()
{
    MyBase* variant;
    uint_8 switchValue = readSwitchValue();

    switch(switchValue)
    {
    case 0:
        variant = new VariantA();
        break;

    case 1:
        variant = new VariantB();
        break;
    }

    variant->run();
}

Now this works just fine. I read the hardware value and use a switch statement to create the new corresponding class.

The problem is that there are a lot of variants I have to deal with. Currently about 15, with the potential to add another 20-30 in the near future. I have really come to despise switch statements that run for hundreds of lines, so I'm really looking for a better way to do this, probably through templates.

I want to be able to use my hardware value to look up a type and use that type to create my new object. Ideally when I add a new variant, I create the new class, add that class type to my lookup table with it's matching hardware value, and it's good to go.

Is this possible at all? What's a good solution here?

Dan
  • 1,805
  • 2
  • 18
  • 21
  • Personally, I think a "switch/case" block to create the appropriate class is probably an optimal solution. Just put your case statement in a static "factory" method that returns a reference to the specific class. IMHO... Here's a good example: http://stackoverflow.com/questions/7468104/factory-method-design-pattern – paulsm4 Apr 16 '13 at 21:40
  • Is the hardware only knowable at runtime? – Kerrek SB Apr 16 '13 at 21:40
  • 1
    have a look at this specific [answer](http://stackoverflow.com/questions/15977617/polymorphism-with-new-data-members/15978673#15978673), which decribe a way to build an object factory by registering constructors. It's probably worth looking up the original idea mentioned in the post. – didierc Apr 16 '13 at 21:41
  • another interesting (and related) concept: dependency injection. – didierc Apr 16 '13 at 21:45
  • @Kerrek, yes the hardware is only knowable at runtime. – Dan Apr 16 '13 at 22:11
  • Despite its title, this question does not seem relevant to [dynamically-typed variables in C++](https://stackoverflow.com/questions/2834139/declaring-a-data-type-dynamically-in-c). – Anderson Green Aug 20 '18 at 17:46

5 Answers5

23

As stated, you make a factory, but not necessarily with naive switch statements. What you can do is make a template class to create the relevant object and dynamically add these to your factory.

class VariantinatorBase {
  public:
    VariantinatorBase() {}
    virtual ~VariantinatorBase() {}
    virtual std::unique_ptr<Variant> Create() = 0;
};

template< class T >
class Variantinator : public VariantinatorBase {
  public:
    Variantinator() {}
    virtual ~Variantinator() {}
    virtual std::unique_ptr<Variant> Create() { return std::make_unique<T>(); }
};

Now you have a class factory that allows you to register these.

class VariantFactory
{
  public:
    VariantFactory()
    {
         // If you want, you can do all your Register() calls in here, and even
         // make the Register() function private.
    }

    template< uint8_t type, typename T >
    void Register()
    {
        Register( type, std::make_unique<Variantinator<T>>() );
    }

    std::unique_ptr<Variant> Create( uint8_t type )
    {
        TSwitchToVariant::iterator it = m_switchToVariant.find( type );
        if( it == m_switchToVariant.end() ) return nullptr;
        return it->second->Create();
    }

  private:
    void Register( uint8_t type, std::unique_ptr<VariantinatorBase>&& creator )
    {
        m_switchToVariant[type] = std::move(creator);
    }

    typedef std::map<uint8_t, std::unique_ptr<VariantinatorBase> > TSwitchToVariant;
    TSwitchToVariant m_switchToVariant;
};

At the beginning of your program, create the factory and register your types:

VariantFactory factory;
factory.Register<0, VariantA>();
factory.Register<1, VariantB>();
factory.Register<2, VariantC>();

Then later, you want to call on it:

std::unique_ptr<Variant> thing = factory.Create( switchValue );
Jan Schultke
  • 17,446
  • 6
  • 47
  • 96
paddy
  • 60,864
  • 6
  • 61
  • 103
  • Sleek. Avoids the need for a static member function that simply does `return new ` in every class and lets the compiler handle the tedious parts. +1 – Nik Bougalis Apr 16 '13 at 21:55
  • Cheers. Might need some tweeks to make it compile - I just hammered this into the answer editor without much thought. But I have used this pattern before and can vouch for its power and simplicity. – paddy Apr 16 '13 at 21:58
  • I should point out that the `Register` function and `Variantinator` types can be private, and you can do all the registration in the factory's constructor. You can also add a little more templating to make this a completely generic factory solution. – paddy Apr 16 '13 at 22:02
  • 3
    +1 I like this design. Some thoughts for improvements: 1. Add a virtual destructor to `VariantinatorBase`. 2. (When using C++11) Make the `Create` methods return `std::unique_ptr` instead of a raw pointer. 3. (When using C++11) Likewise, pass a `std::unique_ptr` to `VariantFactory::Register`. – reima Apr 16 '13 at 22:06
  • Thanks @reima. I think the person using the factory should be able to choose between `unique_ptr` and `shared_ptr`. Surely these can be assigned after `Create` returns a raw pointer. After all, we are using `Create` as a wrapper to `new`. – paddy Apr 16 '13 at 22:14
  • 3
    @paddy You can still choose to transform the returned `unique_ptr` into a `shared_ptr` if you want. Returning a `unique_ptr` is a clear indication that the ownership of the pointer is transferred to the caller and prevents memory leaks. – reima Apr 16 '13 at 22:18
  • Cool, thanks for that. A disclaimer. I don't have much experience with those because I mostly maintain code on older versions of C++. I chucked it in there so I didn't have to make a destructor for `VariantFactory`! ;-) I'll change those all to `unique_ptr` and hope for the best =) – paddy Apr 16 '13 at 22:23
  • Cool, I will try this out when I get back to work in the morning. – Dan Apr 16 '13 at 22:29
  • 2
    One may of course reduce the code duplication down to just `Register(0);` if a template `Register` function is allowed to know about `Variantator`. – Ben Voigt Apr 17 '13 at 01:43
  • 1
    @paddy It's not quite as simple as replacing all raw pointers by `unique_ptr`s. There is no implicit conversion from raw pointer to `unique_ptr` and `unique_ptr`s cannot be assigned to one another (you have to `move` or `swap` them). It should look somewhat like this: http://pastie.org/7628666 – reima Apr 17 '13 at 10:44
  • @reima Okay thanks, I'll edit that in later. That syntax for registration is diabolical, and a good argument for adopting Ben Voight's suggestion for a templated registration function. Do you also need the `move` on returning from `VariantFactory::Create(uint_8)` or is that taken care of by RVO? – paddy Apr 17 '13 at 18:59
  • 2
    Hah, I finally came back to this 5 years later after linking it to someone, and made the required changes. – paddy Oct 24 '18 at 04:24
  • `TSwitchToVariant`'s key has to be POD or not? Would this affect performance somehow if I'd replace `uint_8` with `char[]` or `string`? – im_infamous Jul 01 '19 at 06:59
  • @im_infamous it does not have to be POD. If you replaced it with a char array, you would need to define a custom comparator for the `std::map` of course. And sure, using a string instead of a single integer is generally less performant. But the fact that you are even asking this suggests to me that the performance impact will make zero actual difference to your program. If you're concerned, you could use `std::unordered_map` instead. But be wary of overthinking it. How many millions of factories do you actually intend to be invoking per second? The overhead is more likely to be in `new`. – paddy Jul 08 '19 at 02:59
3

You are looking for a factory

http://www.oodesign.com/factory-pattern.html

A factory is a software module (a method, a class) whose sole purpose is to create the right object for the job. An example using a factory class:

class VariantFactory
{
    MyBase* CreateObject(uint_8 value);
}

And the CreateObject method can be filled out to give you the type of object that you need.

In the case of a very small selection of objects with simple construction, a simple switch statement might suffice. As soon as you get a lot of objects or ones that require more detailed construction, a factory is quite useful.

Silas
  • 406
  • 2
  • 11
  • There's more to it than this, since you're pretty much just moving the switch statement from `main` to `VariantFactory::CreateObject` – Ben Voigt Apr 16 '13 at 21:41
  • Ben, true - which is why I linked to the pattern itself, and added a few further comments. – Silas Apr 16 '13 at 21:42
  • I don't see how this is any better because there will still be a massive switch statement inside the factory. I just came across this thread which has a similar question and interesting answers. http://stackoverflow.com/questions/11831284/dynamic-mapping-of-enum-value-int-to-type – Dan Apr 16 '13 at 21:44
2

I made this a comment; let's turn it into an answer:

Personally, I think a "switch/case" block to create the appropriate class is probably an optimal solution. Just put your case statement in a static "factory" method that returns a reference to the specific class. IMHO...

Here's a good example: factory method design pattern

Class Book : public Product
{
};

class Computer : public Product
{
};

class ProductFactory
{
public:
  virtual Product* Make(int type)
  {
    switch (type)
    {
      case 0:
        return new Book();
      case 1:
        return new Computer();
        [...]
    }
  }
}

Call it like this:

ProductFactory factory = ....;
Product* p1 = factory.Make(0); // p1 is a Book*
Product* p2 = factory.Make(1); // p2 is a Computer*
// remember to delete p1 and p2

Note that in his most excellent response, smink also suggests some other design alternatives, too.

BOTTOM LINE: There's nothing inherently "wrong" with a switch/case block. Even for a switch with many case options.

IMHO...

PS: This really isn't creating a "dynamic type". Rather, it's "creating a static type dynamically". That would be equally true if you used a template or an enum solution as well. But again - I vastly prefer the "switch/case".

Community
  • 1
  • 1
paulsm4
  • 114,292
  • 17
  • 138
  • 190
  • There's nothing wrong with this (or with `switch`) but personally, I find this tedious and error prone. – Nik Bougalis Apr 16 '13 at 21:49
  • Agreed. Somewhere the code has to decide what kind of object to create, and the simplest way to do that is with an if-else chain or a switch statement. Sure, you can create a registry hide the decision in lookup code, but ultimately that obscures what's going on. – Pete Becker Apr 16 '13 at 22:08
2

Update: I am leaving my original solution here for posterity, but consider the solution provided by paddy to be superior and less error prone. With only a couple of slight improvements I think it's actually about as good as you can possibly get.


Consider this design:

class VariantA : public MyBase
{
    static MyBase *CreateMachineInstance() { return new VariantA; }
};

class VariantB : public MyBase
{
    static MyBase *CreateMachineInstance() { return new VariantB; }
};

Now, all you need is an std::map that uses a uint_8 as the key and maps it to a function pointer (returning MyBase). Insert the identifiers in the map (pointing each to the appropriate machine creation function) and then read the code and just use the map to find what machine you're using.

This is loosely based on a concept/pattern called a "factory" but may break slightly if your machine constructors require different arguments or you need to perform additional per-machine initialization/operations - and from what you mention it sounds like you might.

If that's the case, you can still use this pattern but you will have to make some tweaks and rearchitect things a bit but you will end up with something much cleaner and easier to augment and maintain.

Nik Bougalis
  • 10,495
  • 1
  • 21
  • 37
-4
#include <stdio.h>
#include <string.h>
#include <iostream>
using namespace std;

template<class T,class T1>
class HeroHonda
{
private:
    T millage;
    T1 *options;

public:
    HeroHonda() {
        puts("constructed");
        options=new T1[20];

        strcpy(options,"Good millage,Powerstart");
        millage=110;
    }

    virtual T features() {
        cout<<options<<"millage is"<<millage<<endl;
        return 1;
    }

    // virtual T Extrafeatures() = 0;

    ~HeroHonda() {
      cout<<"destructor"<<endl;
      delete [] options;    
    }
};

int main()
{
    HeroHonda <int,char> *Ptr=new HeroHonda <int,char>;
    Ptr->features();
    delete Ptr;
}
Petr B.
  • 879
  • 1
  • 8
  • 17