sud03r
sud03r

Reputation: 19749

Re-design suggestions for the attached code segment

My program needs to run like this:

./myprogram inputType [Can be I1, I2 or I3]

Most of the functions of this program are like:

void Foo::foo (IType inputType) {
    // Some common code
    if (inputType == I1) ... // some I1 specific code
    if (inputType == I2) ... // Some I2 specific code
    ...// similarly for I3
}

These checks of inputType are scattered over multiple places and with time have became increasingly difficult to manage. I have thought of refactoring this code to something like :

InputType* iType = new InputTypeI1(); // or I2 or I3

void Foo::foo (IType inputType) {
    // Some common code
    iType.DoSomething(this, arg1, arg2,..)
}

class InputType1 : public InputType 
{
     // Virtual functions.. (with default implementations)
}

InputType1::DoSomething(Foo* f, Arg1* arg1, Arg2* arg2)
{
    f->DoSomethingFor1(arg1, arg2);
}

This results in organizing things for I1, I2 or I3 as well as automatically call relevant function based upon the input type. However, I feel this could be done better. Any suggestions?

Upvotes: 0

Views: 56

Answers (2)

billz
billz

Reputation: 45410

your current code couples Foo and InputType:

  1. Foo creates InputType Object
  2. InputType calls Foo function

Suggested solution is:

 1. Decouple InputType and Foo by using composites mode
    Foo could hold a pointer to `InputType*` then call InputType `virtual` function.    
 2. To make InputType, a factory will simple enough. 

Sample code:

class InputType
{
 public:
    virtual ~InputType();
    virtual void DoSomething();
};

InputType* MakeInputObject(const IType& inputType)
{
   return new InputTypeX; 
}

class Foo
{
public:
  Foo(const InputType& input) : input_type_ptr(MakeINputObject(input) {} 
  void DoSomething() { input_type_ptr->DoSomeThing(); }

private:
  std::unique_ptr<InputType> input_type_ptr;
};

Upvotes: 1

Roddy
Roddy

Reputation: 68023

Difficult to tell from the snippet you provide, but I'd consider having three derived foo classes, FooI1, FooI2 and FooI3, and constructing the appropriate one with a factory based on InputType.

Then all the specializations just get implemented in virtual methods for each of the new classes.

class FooI1: public Foo {
 void doSomething() {...};
}

ditto for I2/I3..

Foo * fooFactory(InputType iType) 
{ 
   return new FooX - depending on iType
};
Foo *f = fooFactory(i)

f->doSomething();

Upvotes: 1

Related Questions