How to avoid repeating code?

[werasm]

Dear All,

Assume there are three classes where CA has members of class CA1 and
CA2 as follows. To make the public functions of CA1 and CA2 can work on
the members a1 and a2 in a CA object, I just write all the functions
such as a1_func1(), a1_fun2(), a2_func1(), as_func2() in the CA
interface. I think this is a stupid way since if there are more
functions in CA1 and CA2, I need to repeat them all. Is there any good
way to implement it?

Simple solution would be to use the composite pattern (This looks like
it may be a candidate):

Example:
#include <vector> //or list
#include <memory> //for auto_ptr...

namespace nsa{
template <class DeleterT>
struct Deleter
{
void operator()( DeleterT*& lval )
{
delete lval;
lval = 0;
}
};

struct Leaf
{
virtual void f() = 0;
virtual ~Leaf(){ }
};

struct CA1 : public Leaf
{
void f(){ /* Some implementation */ }
};

class CA2 : public Leaf
{
void f(){ /* Some implementation */ }
};

class Composite //CA!!!
: public Leaf //Keep interface same
{
public:
typedef std::auto_ptr<std::vector<Leaf*> > LeaveT;

Composite( LeaveT leaves )
: leaves_( leaves.get() ?
leaves : LeaveT(new std::vector<Leaf*>) )
{
}

//void add( Leaf* ){ /*...*/ }
//void remove( Leaf* ){ /*...*/}

void f()
{
std::for_each( leaves_->begin(), leaves_->end(),
std::mem_fun( &Leaf::f ) );
}
~Composite()
{
std::for_each( leaves_->begin(), leaves_->end(), Deleter<Leaf>()
);
}
private:
LeaveT leaves_;
};

}//nsa

int main()
{
Composite::LeaveT leaves( new std::vector<Leaf*> );
leaves->push_back( new CA1 );
leaves->push_back( new CA2 );
Composite ca( leaves );
ca.f(); //Calls CA1.f() and CA2.f()...
}

I appreciate your help.

Hope it helps. For anybody who wants to know, the reason I'm using
auto_ptr is for ownership transferral, that leaves only require to be
created once. Some boost utility can be used to create the sequence
more elegantly than using push_backs.

Shuisheng

OP ignore question below:

Anybody have a compile time solution? Using valuelists
(boost::inherit_linearly) perhaps. I would like to see?

Regards,

Werner

 

[werasm]

shuisheng wrote:

Make CA1 and CA2 public.

Your current plan only pretends they are private. Users of CA are aware of
them, so they are /de-facto/ public.

Hmmm, program for the future tense.
- Do we want to in future, always expose both CA1 and CA2's entire
interface, especially considering the fact that their interface may
consist of more than func1/func2 at a later stage (even though it is
narrow at present)?
- Would calling CA::a1_func1() in future, always imply that
CA1::func1() is actually called? Maybe in this trivial example, yes.
- What you are saying may be true if interfaces are extremely concise
(only contain one function), and will remain so indefinitely. Even
then, this makes the caller dependend on an interface other than the
interface he is working with currently. Following your (so much
referred to) principles (DIP), the caller won't even be exposed to the
interface of CA, so how would he be able to access members CA1 and CA2?
If using the less liked pimpl, however - well, hide the detail.

In your case, CA1 and CA2 are real objects, not primitive data variables.
Abusing them has less consequences.

Disagree, one only wants to expose necessary interface. What about
interface that in future may become necessary. Abusing them (IMO) has
even more consequence.

If you simply must encapsulate, then write get() methods that return CA1
const & and CA2 const &.

Do we want to expose the entire const interface, or make explicit that
which is necessary?

Alternately, don't allow users of CA to even know of CA1 and CA2's
existence. CA should only accept high-level requests to do things, and it
should figure out for itself how to do them. Interfaces should tell, not
ask, CA to do its tasks.

Yes (the preferred choice), but not alternative to making them public.

Kind regards,

Werner

 

[Phlip]

Disagree, one only wants to expose necessary interface.

This aspect of the thread has been thoroughly discussed. I try not to
practice coding techniques that prevent future upgrades. Sometimes this
looks like a little slack in the coding style. It is not, and fixes could
easily include privatizing the two variables, then trivially fixing every
resulting syntax error.

Another form of Premature Optimization is premature publication. Don't
pretend you are writing a library that millions of user-programmers will
use, hence don't over-design your library such that no upgrade can ever
cause a global syntax error. Just wait till it happens (it might not), then
lean on the compiler, fix it, and keep going.

 

[werasm]

This aspect of the thread has been thoroughly discussed.

I thought I might have shed some new light on it . Maybe I was
wrong. I know you have lot of experience (read your stuff, or some of
it). I would rather teach the OP the principle of keeping data members
private than not (as opposed to you, I think). BTW (OT), I think in
some cases "Appeal to authority" can be a good thing. Especially when
one has to do something and you don't have the mental capability (due
to lack of experience, perhaps). Following good authorities mechanisms
often get you out of trouble at the outset. Later only, you understand
why. 80% of the time appeal to good authority can save ones bacon.
Often 80% is all you need to be homefree for ever :-...

I try not to
practice coding techniques that prevent future upgrades. Sometimes this
looks like a little slack in the coding style. It is not, and fixes could
easily include privatizing the two variables, then trivially fixing every
resulting syntax error.

Not convincing. Unfortunately my compiler does not force me to make
that change (privatizing) when the interface of the classes whom I
aggregate (or use) change. If I use this rationale consistently in
large projects, it would just be a matter of time before things become
unmanageable. Rather "implement in terms of" and only expose the
necessary interface pertaining to the services I provide.

Another form of Premature Optimization is premature publication. Don't
pretend you are writing a library that millions of user-programmers will
use, hence don't over-design your library such that no upgrade can ever
cause a global syntax error. Just wait till it happens (it might not), then
lean on the compiler, fix it, and keep going.

In this case, the problem is not that upgrades will cause a global
syntax error, but that upgrades won't cause a global syntax error and
that type A, containing type B, given that type A's state may be
affected by type B, may have its state influenced from externally (as
types B's interface is exposed). Then we also have the argument that
now client has to learn both A and Bs interface etc. I agree that
Premature Optimization (or the other form of it) should be avoided. One
has to discern though, between this and doing something right the first
time. You follow many principles I'm sure, that when looking sternly
upon them, one could consider them to be premature as well. It does not
prevent you from following them, as software does have the tendency to
change more often than less (as you of all people know). All in all you
do try and create hinge points prematurely at places which you
anticipate could change, don't you - ever?

If I follow one little principle consistently, which is...

Make datamembers private, and member functions private unless they
contribute to the functional interface.

.... I seem to have less problems (when I follow this principle) than
when I do not (IME). For this reason we will have to agree to disagree
.

Kind regards,

Werner

 

[Noah Roberts]

The purpose
of making things private is as much to keep the containing object in a
consistent state as it is to keep the contained objects in such a state.

In my opinion it is more about allowing change. Objects that expose
their internals create rigidity. You'll find it much more difficult to
alter a class if it's internals are open to everyone.