Design a better API interface to pass a struct from one class to another class [closed]

Question

I am a strong believer of the following design philosophy:

1> Services should be implemented as close as possible where the data is stored.

2> Getter and Setter are evil and should be used carefully.

I rather NOT argue above two arguments here and assume they have their edges.

Here is the challenge I am current facing. I have two classes (i.e AComputer and A) where AComputer provides some services for A and A holds all fundamental data members.

Fact: I am not allowed to combine AComputer inside A due to the system design. I knew, it has broken my point 1> where computation should stay with data.

When passing data from A to AComputer, we have to pass 10(approximately) individual parameters and so it is better to design a structure to do that otherwise the constructor list will grow crazy. Most of the data stored in AComputer are direct copies those of stored in A. We chose to store those data inside AComputer because other functions in AComputer also need those variables.

Here is the question( I am asking for best practice considering API maintenance & modification):

1> Where should we define the pass-structure PassData?

2> Should we provide getter/setter for struct PassData?

I have provided a sample code as follow to illustrate my question in details. It is best that I can find a real work open-source API that has addressed the same issue so that I can learn from it.

If you look at private PassData m_data; defined in class AComputer, I do this in purpose. In other words, if we change the underlying implementation of AComputer, we can replace PassData m_data; with individual variables or something else but NOT break the interface of PassData. So in this design, I do NOT provide a getter/setter for the struct PassData.

Thank you

class AComputer
{
public:
    struct PassData
    {   // int type just used as an illustration. Real data has different types,
        // such as double, data, string, enum, etc.
        // Note: they are not exact copies of variables from A but derived from them
        int m_v1;
        // from m_v1 to m_v10
        //...
        int m_v10;
    };

    // it is better to store the passed-in data since other functions also need it.
    AComputer(const PassData& pd) : m_data(pd) {}

    int GetCombinedValue() const
    { /* This function returns a value based the passed-in struct of pd */ }

private:
    PassData m_data;    
};

class A
{
private:
    int m_i1;
    // from m_i1 to m_i10
    // ...
    int m_i10;
    // from m_i11 to m_i20
    // ...
    int m_i20;

    boost::shared_ptr<AComputer> m_pAComputer;

public:
    A()
    {
        AComputer::PassData aData;
        // populate aData ...
        m_pAComputer = boost::shared_ptr<AComputer>(new AComputer(aData));
    }

    int GetCombinedValue() const
    {
        return m_pAComputer->GetCombinedValue();
    }
};

Answer 1

I think it is better clarify a couple of points before start, you said:

If you look at private PassData m_data; defined in class AComputer, I do this in purpose. In other words, if we change the underlying implementation of AComputer, we can replace PassData m_data; with individual variables or something else but NOT break the interface of PassData.

This is not true, PassData is part of your interface! You cannot replace PassData without breaking client code, because you require PassData in the constructor of AComputer. PassData is not implementation details, but it is pure interface.

Second point that needs clarification:

2> Getter and Setter are evil and should be used carefully.

Correct! But you should know that a POD (Plain-Old-Data struct) is even worst. The only advantage of using a POD instead of class with getter and setter is that you save the trouble to write the functions. But the real issue is still open, the interface of your class is too cumbersome and it will be very difficult to maintain.

Design is always a trade-off between different requirements:

A false sense of flexibility

Your library is distributed and a lot of code is using your class. In this case a change in PassData will be dramatic. If you can pay a small price in runtime you can make your interface flexible. For example the constructor of AComputer will be:

AComputer(const std::map<std::string,boost::any>& PassData);

Have a look at boost::any here. You may also provide a factory for the map, in order to help the user to create the map easily.

Pro

• If you do not required a field any more the code is unchanged.

Cons

• Small runtime price.
• Lose the compiler type safe check.
• If your function requires another mandatory field you are still in trouble. The client code will compile but it will not behave correctly.

Overall this solution is not good, at the very end it is just a fancy version of the original one.

Strategy Pattern

struct CalculateCombinedValueInterface
{
   int GetCombinedValue()=0;
   virtual ~CalculateCombinedValueInterface(){}
};

class CalculateCombinedValueFirst : CalculateCombinedValueInterface
{
   public:
       CalculateCombinedValueFirst(int first):first_(first){}
       int GetCombinedValue(); //your implementation here
   private:
       //I used one field but you get the idea
       int first_;
};

The client code will be:

CalculateCombinedValueFirst* values = new CalculateCombinedValueFirst(42);

boost::shared_ptr<CalculateCombinedValueInterface> data(values);

Now, if you are going to modify your code, you shouldn't touch already deployed interface. The Object Oriented solution for this is provide a new class that inherites from the abstract class.

class CalculateCombinedValueSecond : CalculateCombinedValueInterface
{
   public:
       CalculateCombinedValueFirst(int first,double second)
           :first_(first),second_(second){}
       int GetCombinedValue(); //your implementation here
   private:
       int first_;
       double second_;
};

The client will decide if upgrade to the new class or to stay with the existing version.

Pro

• Improve your interface without break client code.
• You are not touching existing code, but you introduce new functionality in a new file.
• You may want to use the template method design pattern if you want a smaller granularity control.

Cons

• Overhead of using virtual functions (basically few picoseconds!)
• You cannot break existing code. You have to leave the existing interface untouched and add a new class to model different behavior.

Number of parameters

If you have a set of ten parameters input in one function, it is very likely that these values are logically related. You may collect some of these values in classes. Those classes may be combined in another class that it will be the input of your function. The fact that you have 10 (or more!) data members in a class should ring a bell.

The single responsibility principle said:

There should never be more than one reason for a class to change.

The corollary of this principle is: your class has to be small. If your class has 20 data member is very likely that you will find a lot of reason to change it.

Conclusion

After you have provided an interface (any kind of interface) to the client you cannot change it (a good example are all the deprecate features in C++ that compilers need to implement for years). Pay attention at the interface that you are providing even implicit interface. In your example, PassData is not implementation details but it is part of the class interface.

The number of parameters is a signal that your design needs to be reviewed. It is very difficult change a big class. Your classes should be small and depend to other classes only via an interface (abstract class in C++ slang).

If your class is :

1) small and with just one reason to be changed

2) derived from an abstract class

3) other classes refer to it using a pointer to the abstract class

Your code can be changed easily (but the already provide interface has to be preserved).

If you do not fulfill all these requirements, you will be in trouble.

NOTE: requirements 2) and 3) can change if instead of provide dynamic polymorphims the design is using static polymorphims.