问题描述:

In my code, there's a base class A which has three subclasses A1, A2, A3. These objects have a state to specify if they are active or not.

Now in another class B , objects of A1, A2, and A3 are created. Now in a function inside B,

int B::SetState( A& aObj)

{

//if aObj is an instance of A1, do nothing and return.

//if aObj is an instance of A2, do something if A3 is active

//if aObj is an instance of A3, do something if A2 is active

//the code below i would like to change to something more maintainable

if ( aObj.GetNumber() == 0 )

return;

A& otherAObj = aObj.GetNumber()==1? GetAObj(2) : GetAObj(3);

if ( otherAObj.IsActive() )

{

//do something with aObj

}

}

I would like to change the code to something like aObj.DoSomething() but the dependency between sibling is making it tough. Any other good ideas to refactor? The class structure has to remain the same.

网友答案:

You can refactor this:

if ( otherAObj.IsActive() )
{
  //do something with aObj
}

out into a doSomething() function

public class A{
    virtual bool isActive(){ return false; }
    virtual bool isPending(){ return false; }
    virtual void doSomething()
    {
        if(true == isActive())
        {
            ...
            if(false == isPending())
            { ... }
            ...
        }
    };
}

public class A_State_Active : A
{
    bool isActive(){ return true; }
}

public class A_State_Pending : A
{
    bool isPending(){ return true; }
    void doSomething()
    {
        throw new InvalidOperationException("Don't be messing");
    }
}

Or even make the base version of doSomething pure virtual to force the users to implement a state specific version.

However you should not implement anything in your subclasses that is based on the state of a different object. IMO you should use a manager type object in that case. If really want to do it that way you should just pass a reference to the other objects to the doSomething() function and modify the runtime behaviour based on the return values of the passed object's state (query using the "isActive"/"isPending" API).

Other option is to do away with inheritance and favour composition. Have A be one class which holds a reference to an A_State class. You then subclass A_State.

class A
{
    //public functions
    ...
    const A_State* getState(){ return m_poMyState; }
    void setState(const A_State const * aStateObj ){ m_poMyState = aStateObj; }
}
网友答案:

You could enhance the A interface with functions like

virtual bool hasSibling() const = 0;
virual int siblingNumber() const = 0;

Then, given suitable implementations in the subclasses the B code can do:

if (!aObj.hasSibling()) return;
A & otherObj = GetAObj(aObj.siblingNumber());
if (otherObj.isActive()) { ... }

It'd be better if you could push some more of the logic down into A (on the tell don't ask principle), but that may be tricky with GetAObj residing on B.

相关阅读:
Top