views:

243

answers:

6

Suppose I have a data type enum TreeTypes { TallTree, ShortTree, MediumTree }.

And I have to initialize some data based on one particular tree type.

Currently I have written this code:

int initialize(enum TreeTypes tree_type) {
    if (tree_type == TallTree) {
        init_tall_tree();
    }
    else if (tree_type == ShortTree) {
        init_short_tree();
    }
    else if (tree_type == MediumTree) {
        init_medium_tree();
    }
    return OK;
}

But this is some kind of stupid code repetition. I am not using any of the powerful C++ capabilities like templates.

How could I write this code better?

Thanks, Boda Cydo.

+16  A: 

Your code is OK for two or three values, but you are right, you need something more industrial strength when you have hundreds of them. Two possible solutions:

  • use a class hierarchy, not enums - you can then use virtual functions and have the compiler work out which actual function to call

  • create a map of enum -> function, which you initialise at startup - your function calls then become something like map[enum]->func()

Templates don't work so well here, because you are trying to make a decision at run-time, whereas templates do their stuff at compile-time.

anon
+1, Dynamic dispatch is designed for this sort of thing...
Chris Thompson
The function map can benefit from using `boost::function`/`boost::bind()` or the TR1 equivalents.
Georg Fritzsche
This sounds like a vtable, C++ already has this with virtual functions and polymorphism...
joshperry
@josh Golly, isn't that just what my answer says?
anon
@Neil sorry, misread; I guess my eye was drawn to the map[enum] part.
joshperry
A: 

Try a switch statement:

int initialize(enum TreeTypes tree_type) {
    switch (tree_type) {
        case TallTree: 
            init_tall_tree();
            break;
        case ShortTree:
            init_short_tree();
            break;
        case MediumTree:
            init_medium_tree();
            break;
    }
    return OK;
}
tdammers
No different from if-else, and in fact more code.
anon
A: 

If this initialization is really the only distinction, then I'm not sure any other idiom would improve the situation.

You could subclass from Tree and create the right sort of tree object...but you'd still need to differentiate which one to instantiate, so you'd still wind up with a similar if/else block, somewhere.

That said, if there is more than just initialization that'd different, you should subclass and use virtual functions to enact the differences between them.

Uncle Mikey
+7  A: 

In one word: inheritance

class Tree { public: virtual void initialize() = 0; }

class ShortTree : public Tree {
public:
    virtual void initialize(){
        /* Short Tree specific code here */
    }
}

class MediumTree : public Tree {
public:
    virtual void initialize(){
        /* Medium Tree specific code here */
    }
}

class TallTree : public Tree {
public:
    virtual void initialize(){
        /* Tall Tree specific code here */
    }
}

Then wherever you want to call initialize just make sure to have a pointer or a reference for polymorphism to work correctly:

Vector<Tree*> trees;
trees.push_back(new SmallTree());
trees.push_back(new MediumTree();
trees.push_back(new TallTree();

// This will call the tree specific code for each tree in the vector
for(vector<Tree*>::iterator tree = trees.begin(); tree!=trees.end(); ++tree)
    tree->initialize();
BioBuckyBall
@Justin Ardini my c++ rust is showing. fixed that. thanks
BioBuckyBall
Your base class had better have a virtual destructor, assuming you ever call delete on the things you have new'd.
anon
@Neil Butterworth My code has been taken and run with...I don't recognize it anymore :)
BioBuckyBall
+1  A: 

Use a lookup table that is indexed by the enum values (assuming all of the functions have the same signature), ie:

enum TreeTypes { TallTree, ShortTree, MediumTree, MaxTreeTypes }

typedef void (*p_init_func)(void); 

p_init_func initialize_funcs[MaxTreeTypes] =
{
    &init_tall_tree, 
    &init_short_tree,
    &init_medium_tree
};

int initialize(enum TreeTypes tree_type)
{ 
    initialize_funcs[tree_type]();
    return OK; 
} 
Remy Lebeau - TeamB
A: 

And the template way since you have pointed it in your tags:

enum TreeTypes { Tall, Short, Medium };

struct TreeBase {
    // (...)
};

struct TallTree : public TreeBase {
    // (...)
};

struct ShortTree : public TreeBase {
    // (...)
};

struct MediumTree : public TreeBase {
    // (...)
};

template<TreeTypes N_type = Tall>
struct Tree : public TallTree {
    // (...)
};

template<>
struct Tree<Short> : public ShortTree {
    // (...)
};

template<>
struct Tree<Medium> : public MediumTree {
    // (...)
};

That way you got seperate classes for each tree type which can be accessed by base pointer. Wrapping them into Tree class let you do this:

Tree<Tall> tall_tree;
Tree<Short> short_tree;
Tree<Medium> medium_tree;
erjot