How to avoid repeating code?

S

shuisheng

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?

class CA1
{
public:
void func1( );
void func2( );
...
};

class CA2
{
public:
void func1( );
void func2( );
...
};

class CA
{
public:
void a1_func1( a1.func1( ) );
void a1_func2( a1.func2( ) );
...
void a2_func1( a2.func1( ) );
void a2_func2( a2.func2( ) );
...
private:
CA1 a1;
CA2 a2;
...
}

I appreciate your help.

Shuisheng
 
S

shadowman615

shuisheng said:
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?

class CA1
{
public:
void func1( );
void func2( );
...
};

class CA2
{
public:
void func1( );
void func2( );
...
};

class CA
{
public:
void a1_func1( a1.func1( ) );
void a1_func2( a1.func2( ) );
...
void a2_func1( a2.func1( ) );
void a2_func2( a2.func2( ) );
...
private:
CA1 a1;
CA2 a2;
...
}
I suppose the simplest solution is to make class CA a virtual base
class, with CA1 and CA2 subclasses.

Then you could create a container of pointers to CA...

Without explaining the entire concept, my advice is to read up on
polymorphism for some examples of this.
 
P

Phlip

shuisheng said:
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?

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.
class CA
{
public:
void a1_func1( a1.func1( ) );
void a1_func2( a1.func2( ) );
...
void a2_func1( a2.func1( ) );
void a2_func2( a2.func2( ) );
...
private:
CA1 a1;
CA2 a2;
...
}

I am aware one of the first OO rules we learn is "don't make data members
public". Rules are meant to be made, followed, and broken. In this case, the
real rule behind that rule is "don't make primitive things globally
writable".

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

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

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.
 
N

Noah Roberts

shuisheng said:
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?

class CA1
{
public:
void func1( );
void func2( );
...
};

class CA2
{
public:
void func1( );
void func2( );
...
};

class CA
{
public:
void a1_func1( a1.func1( ) );
void a1_func2( a1.func2( ) );
...
void a2_func1( a2.func1( ) );
void a2_func2( a2.func2( ) );
...
private:
CA1 a1;
CA2 a2;
...
}

I appreciate your help.

Well, it looks like your class is a candidate for removal as it doesn't
seem to do anything but put two object together. On the other hand it
probably does more in ... so lets assume it has a purpose.

This is the best way to do what you want. You could return a1 and let
the client call the function but this creates a smell known as a middle
man or cascade calls. You can't then alter your class to do other
things when that call is needed. It creates rigidity.

Other alternative is inheritance, inheritance also creates dependencies
(more so in fact) so composition is a better choice when possible.
Unless you need to behave polymorphically you don't want to
inherit....don't inherit for the purposes of reuse.
 
N

Noah Roberts

Phlip said:
I am aware one of the first OO rules we learn is "don't make data members
public". Rules are meant to be made, followed, and broken. In this case, the
real rule behind that rule is "don't make primitive things globally
writable".

Seems you don't fully understand that rule or its purpose.
In your case, CA1 and CA2 are real objects, not primitive data variables.
Abusing them has less consequences.

No, it doesn't. It has the same consequences. You cannot change CA so
that it uses different objects or removes one or both CA# objects from
its internals.
If you simply must encapsulate, then write get() methods that return CA1
const & and CA2 const &.

This has the same consequence and is only slightly better than a public
member variable (at least with this you can remove the variable and
just return a newly constructed object...assuming that changes aren't
expected to affect CA).
Alternately, don't allow users of CA to even know of CA1 and CA2's
existence.

Yes, remove the inappropriate intamacy if there is one.
 
S

Siam

shuisheng said:
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?

class CA1
{
public:
void func1( );
void func2( );
...
};

class CA2
{
public:
void func1( );
void func2( );
...
};

class CA
{
public:
void a1_func1( a1.func1( ) );
void a1_func2( a1.func2( ) );
...
void a2_func1( a2.func1( ) );
void a2_func2( a2.func2( ) );
...
private:
CA1 a1;
CA2 a2;
...
}

I appreciate your help.

Shuisheng

If you're not too concerned about the amount of repeated code compiled,
but are simply too lazy to retype the method delegations for every
function, you could use a macro like:

#define DELEGATE(member, function) void member ## _ ##
function(){member.function();}

Now you can replace the function definitions in your class declaration
with something like DELEGATE(a1, func1) for each function you want
delegated. You would call the function in the same way you would have
in your version [ a1_func1() ].

Siam
 
P

Phlip

Noah said:
Seems you don't fully understand that rule or its purpose.

You have such a pleasant manner of posting! I look forward to
pair-programming with you some day...
No, it doesn't. It has the same consequences. You cannot change CA so
that it uses different objects or removes one or both CA# objects from
its internals.

The original design had that problem, as I hinted above. Thanks for
explicating it.
This has the same consequence and is only slightly better than a public
member variable (at least with this you can remove the variable and
just return a newly constructed object...assuming that changes aren't
expected to affect CA).
Right.


Yes, remove the inappropriate intamacy if there is one.

Another way to detect inappropriate intimacy - even of things we have
foolishly made private - is examining the distance between them. If a module
way over there reaches out and grabs a class here - a class not in the
interface of this module, then you have inappropriate intimacy anyway.

One fix is to simply bust your modules apart with dynalink interfaces, ORB
interfaces, or even a soft glue language. Then your modules are small and
incapable of exploiting the C++ linker to go where they are not wanted. And
then the trivial details of what's public and private matter less to each
module.

This topic is "Encapsulation is Hierarchical". Have you heard the term
before?
 
N

Noah Roberts

Phlip said:
You have such a pleasant manner of posting! I look forward to
pair-programming with you some day...

Well, you explained it incorrectly and then gave advice based on that
mistaken definition. There is no "rule behind the rule" that narrows
it to primitive members.

I think there are probably two sides to your little stab with the pair
programming thing.
The original design had that problem, as I hinted above. Thanks for
explicating it.

Yes, the original design had problems but that wasn't what my
explaination was about. My explaination was counter to your claim that
exposing member variables somehow has less concequences if those
variables are not primitive. That simply isn't the case.
 
P

Phlip

Noah said:
Well, you explained it incorrectly and then gave advice based on that
mistaken definition. There is no "rule behind the rule" that narrows
it to primitive members.

That argument is "appeal to authority". No peer-reviewed author you know of
has proposed such a rule.

I appreciate your attention to the entry-level guidelines of programming,
yet knowing their meaning is more important than such knee-jerk reactions to
violating them.

Oh, BTW, the programming author Ward Cunningham sez to relax the private
data rules.
I think there are probably two sides to your little stab with the pair
programming thing.

That is indeed how the expression "I look forward to pairing with you on
that" is used. (According to several authors...)
Yes, the original design had problems but that wasn't what my
explaination was about. My explaination was counter to your claim that
exposing member variables somehow has less concequences if those
variables are not primitive. That simply isn't the case.

They can fend for themselves. They cannot, for example, be driven into an
incorrect state, if _their_ members are private.

They might indeed be driven into a state that CA does not expect. Yet they
might also if they were private. And they might have a reference to the CA,
so they can inform it of their state.

So we are back to the Hollywood Principle again...
 
K

Kaz Kylheku

shuisheng said:
void a1_func1( a1.func1( ) );
void a1_func2( a1.func2( ) );

This isn't valid syntax. Try again.

Do you mean

void a1_func() { a1.func1(); }

?
private:
CA1 a1;
CA2 a2;
...
}

Doh, why don't you just make these members public? Then the user of the
class can write:

a.a1.func();
a.a2.func();

This way you don't have to write any wrapper functions. After all, your
class CA is just an aggregate that holds a1 and a2, so why not open it
up.

The only thing you have accomplished with those wrappers is changing a
period to an underscore:

a.a1_func();

Since a1 and a2 are already class objects, they can use their own
private: specifiers to restrict access.
 
D

Daniel T.

"shuisheng said:
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?

class CA1
{
public:
void func1( );
void func2( );
...
};

class CA2
{
public:
void func1( );
void func2( );
...
};

class CA
{
public:
void a1_func1( a1.func1( ) );
void a1_func2( a1.func2( ) );
...
void a2_func1( a2.func1( ) );
void a2_func2( a2.func2( ) );

The above doesn't compile. I'm going to assume you meant (as others
assumed):
void a1_func1() { a1.funct1(); }
// &c.
...
private:
CA1 a1;
CA2 a2;
...
}

I appreciate your help.

Are all of CA1 and CA2's member-functions simply getting passed through
like that? If no, then leave it, your code is fine. If yes then I have
to wonder what CA's purpose in life is?
 
D

Daniel T.

"Kaz Kylheku said:
This isn't valid syntax. Try again.

Do you mean

void a1_func() { a1.func1(); }

?


Doh, why don't you just make these members public? Then the user of the
class can write:

a.a1.func();
a.a2.func();

This way you don't have to write any wrapper functions. After all, your
class CA is just an aggregate that holds a1 and a2, so why not open it
up.

The only thing you have accomplished with those wrappers is changing a
period to an underscore:

a.a1_func();

Since a1 and a2 are already class objects, they can use their own
private: specifiers to restrict access.

Your assuming that all functions of a1 and a2 should be available to
clients of CA and that may not be the case.
 
P

Phlip

Daniel said:
You're assuming that all functions of a1 and a2 should be available to
clients of CA and that may not be the case.

In that situation, CA1 and CA2 can befriend CA, who then only accesses the
secret methods.
 
S

shuisheng

Just give an example.

Assume I have a device which has several critical points to define the
device's pose. Those points are movable, rotatable along an axis and so
on.

class CPoint
{
public:
void Move(double displacement[3]);
void Rotate(double angle, double axis[3]);
private:
double x, y, z;
}

class CDevice
{
private:
CPoint pt1, pt2, pt3;
}

So to control the device's critical points, I have to repeat writing
the Move and Rotate operations on every critical point???
 
D

Daniel T.

"Phlip said:
In that situation, CA1 and CA2 can befriend CA, who then only accesses the
secret methods.

Now you are assuming that only CA should be allowed to call those
methods in all situations. That may not be the case either.

If CA's job is to maintain some invariant between its a1 and a2
variables, then it wouldn't make sense to make member-functions of CA1
and CA2 private, because they would only be private in the CA context
but not other contexts.
 
P

Phlip

shuisheng said:
Assume I have a device which has several critical points to define the
device's pose. Those points are movable, rotatable along an axis and so
on.

class CPoint
{
public:
void Move(double displacement[3]);
void Rotate(double angle, double axis[3]);
private:
double x, y, z;
}

class CDevice
{
private:
CPoint pt1, pt2, pt3;
}

So to control the device's critical points, I have to repeat writing
the Move and Rotate operations on every critical point???

I imagine this problem resembles an arm, like the "robotic" arm on the Space
Shuttle.

Here is a deliberately naive test case:

TEST(reach)
{
Point reachTo(42,10,1);
CDevice aDevice;
aDevice.reach(reachTo);
assert(Point(3,2,0.4) == aDevice.getPoint1());
assert(Point(6,5,0.7) == aDevice.getPoint2());
assert(Point(41,9,0.9) == aDevice.getPoint1());
}

I am guessing that pt1, pt2, and pt3 represent the shoulder, elbow, and
wrist of this robotic arm. (I'm aware that might not be what you mean. This
is just an example.)

Download UnitTest++, and use it to write tests like that, to simplify your
development.

My test shows that I don't tell the device How to reach to the point
[42,10,1]. I only tell the device Reach!, and where to reach to.

The device itself positions its own shoulder, elbow, and wrist such that the
hand at the end is at the point requested.

I am using this test case only as a design technique; as a way to consider
what I want the interface to do, in terms of its clients. If I know that
clients of this device only need to know the Reach point, then I can specify
the interface, simply, like this. pt1, pt2, and pt3 need only expose their
Move() operations to the Device, not to the whole world. Encapsulation is
hierarchical.

So the question for you: What do the clients of your device need it to do?
How do they need it to appear, to them? What do they look like, what do they
know, and what's the simplest interface on Device that will satisfy them?
 
P

Phlip

Daniel said:
Now you are assuming that only CA should be allowed to call those
methods in all situations. That may not be the case either.

If CA's job is to maintain some invariant between its a1 and a2
variables, then it wouldn't make sense to make member-functions of CA1
and CA2 private, because they would only be private in the CA context
but not other contexts.

The goal, in general, is to give the entire capsule around CA, CA1, and CA2
the smallest surface area possible. Sometimes friendship can help with that.
 
D

Daniel T.

"Phlip said:
The goal, in general, is to give the entire capsule around CA, CA1, and CA2
the smallest surface area possible. Sometimes friendship can help with that.

Agreed, but you are assuming that CA1 and CA2 some kind of internal
utility classes that only CA uses. What if they are 'vector' and 'list'?
 
P

Phlip

Daniel said:
Agreed, but you are assuming that CA1 and CA2 some kind of internal
utility classes that only CA uses. What if they are 'vector' and 'list'?

My goodness, Danny, then they shouldn't befriend CA! Among other reasons,
they have closed source!

However, if they were vector and list, then CA should not expose them, not
even by an accessor to a constant reference. 3rd party classes have
super-wide interfaces, and we want to narrow the interfaces around the
region of CA, right?
 
D

Daniel T.

"shuisheng said:
Just give an example.

Assume I have a device which has several critical points to define the
device's pose. Those points are movable, rotatable along an axis and so
on.

class CPoint
{
public:
void Move(double displacement[3]);
void Rotate(double angle, double axis[3]);
private:
double x, y, z;
}

class CDevice
{
private:
CPoint pt1, pt2, pt3;
}

So to control the device's critical points, I have to repeat writing
the Move and Rotate operations on every critical point???

No. The Device is supposed to control its own critical points. Move and
Rotate shouldn't be accessible to CDevice's clients at all.

Presumably CDevice has some invariants between pt1, pt2 and pt3 (if it
doesn't, then I have to wonder why it exists.) For example "distance(
pt1, pt2 ) - 23.0 < sigma". Obviously, if this is true, it would be
dangerous to allow clients of CDevice access to those points.

BTW, this is a *great* example. It shows exactly why other peoples
suggestion of making the contained objects public is a bad idea.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top