Best design for my classes to avoid code duplication?

J

Jef Driesen

Hi,

I have the following problem. I have an interface (abstract base class)
to represent a "device":

class IDevice {
virtual Read () = 0;
}

I have a number of concrete devices, that implement this interface. The
typical implementation of a device consist of a protocol part (defines
how to read the data) and a layout part (defines the structure of the
data and thus where to read the data):

class Device : IDevice {
m_protocol;
m_layout;
...
}

Device::Device ()
: m_protocol (...), m_layout (...)
{
...
}

Device::Read ()
{
return m_protocol->Read (m_layout->offset);
}

But now I want to implement some devices which are very similar. For
instance two devices sharing the same layout, but using a different
protocol. What is the preferred way to implement this, without
duplicating all code?

Basically, only the constructor is different. All other member functions
(such as Read) have exactly the same implementation.

ADevice::ADevice ()
: m_protocol (new AProtocol (...)), m_layout (new Layout (...))
{
...
}

BDevice::BDevice ()
: m_protocol (new BProtocol (...)), m_layout (new Layout (...))
{
...
}

Thanks in advance,

Jef
 
M

McColgst

Basically, only the constructor is different. All other member functions
(such as Read) have exactly the same implementation.

If I understand this problem correctly, you might be able to put an
optional parameter in your constructor which identifies which way to
construct the object, depending on whichever device you need to use.
 
S

Sherm Pendley

If I understand this problem correctly, you might be able to put an
optional parameter in your constructor which identifies which way to
construct the object, depending on whichever device you need to use.

Yeah. If you want to win at amateur hour.

sherm--
 
J

Jerry Coffin

[ ... ]
But now I want to implement some devices which are very similar.
For instance two devices sharing the same layout, but using a
different protocol. What is the preferred way to implement this,
without duplicating all code?

Basically, only the constructor is different. All other member functions
(such as Read) have exactly the same implementation.

From the looks of things, you could use a template:

template <class protocol, class layout>
class Device {
protocol m_protocol;
layout m_layout;
public:
Device() : m_protocol(new protocol), m_layout(new layout) {}
};

Device<AProtocol, ALayout> ADevice;
Device<BProtocol, BLayout> BDevice;

It doesn't sound like you need the other possibility, which would be
that you don't know the combination of protocol and layout until run
time. If you did need to do that, you'd probably want the device to
store a reference (or pointer) to its associated layout and protocol,
and have a factory class to look at the data and create a device
object with the correct layout and protocol for that data.
 
B

Balog Pal

Juha Nieminen said:
Isn't that a bit of a far-fetched solution for something which is
exactly what inheritance was invented for?

far-fetched? The request mapped perfectly to what templates do, and Jerry
showed the code that gives the perfect solution (to what was stated).

Inheritance was invented for a different thing, and would solve this case in
a contorted way if at all -- and OP seem to be familiar with it enough, and
turned here discovering the redundancy it leaves.
He wants classes with common functionality, except for the
constructor, which is specialized for each class. That's exactly what
inheritance is for.

Care to show an example?
And in current C++ you can not inherit constructors, so how that would be
handled?

The classes' "common functionality" is really common just textually -- it
invokes different functions on objects of different types.

Inheritance could deal with the "different function" thing calling a
virtual, but is hosed with the different member.

Certainly the class could have parts that are
 
J

Jef Driesen

That's exactly what inheritance was invented for. You create a base
class which contains the common functionality, and then specialize from
it. In your case, it would be something like this (rename the classes to
more descriptive names):

class IDeviceWithCommonFunctionality: public IDevice
{
public:
virtual Read(); // The common implementation
};

class ADevice: public IDeviceWithCommonFunctionality
{
public:
ADevice(); // constructor
};

class BDevice: public IDeviceWithCommonFunctionality
{
public:
BDevice(); // constructor
};

The problem with this approach is that the common code needs access to
the protocol and layout. And if those are made members of the concrete
classes, the common classes don't have access to them. But if you store
them in the common class, the common class can't initialize them
properly, because only the concrete classes have that knowledge to
choose the appropriate protocol and layout.
 
J

Jef Driesen

[ ... ]
But now I want to implement some devices which are very similar.
For instance two devices sharing the same layout, but using a
different protocol. What is the preferred way to implement this,
without duplicating all code?

Basically, only the constructor is different. All other member functions
(such as Read) have exactly the same implementation.

From the looks of things, you could use a template:

template<class protocol, class layout>
class Device {
protocol m_protocol;
layout m_layout;
public:
Device() : m_protocol(new protocol), m_layout(new layout) {}
};

Device<AProtocol, ALayout> ADevice;
Device<BProtocol, BLayout> BDevice;

It doesn't sound like you need the other possibility, which would be
that you don't know the combination of protocol and layout until run
time. If you did need to do that, you'd probably want the device to
store a reference (or pointer) to its associated layout and protocol,
and have a factory class to look at the data and create a device
object with the correct layout and protocol for that data.

Templates are not an option in my case for two reasons:

First, the constructor for different protocol/layout objects can be
different (e.g. different number of arguments, different types, etc),
and the code in the constructor is also not the same for each device.

Second, my actual implementation is not in C++, but in plain C (for
portability reasons). I do have an object oriented framework where
inheritance and polymorphism is done with structs and manual vtable's,
but templates is simply not possible. I posted to a C++ newsgroup
because I design in C++ and then port the concept to my C environment.
It's easier to visualize the concept without the C oop boilerplate code.
 
T

Thomas J. Gritzan

Am 09.01.2010 20:14, schrieb Jef Driesen:
The problem with this approach is that the common code needs access to
the protocol and layout. And if those are made members of the concrete
classes, the common classes don't have access to them. But if you store
them in the common class, the common class can't initialize them
properly, because only the concrete classes have that knowledge to
choose the appropriate protocol and layout.

Read about Dependency injection, i.e.:
http://en.wikipedia.org/wiki/Dependency_injection

In short, you pass a pointer/reference of a specific service
implementation to a class, so that the class doesn't have to create it,
and doesn't even have to know about it's existance (only the abstract base).

class BasicDevice : public IDevice
{
public:
virtual Read() { /* ... */ }; // common implementation

protected:
// this class can't be instantiated directly
BasicDevice(layout& l, protocol& p)
: m_layout(&l), m_protocol(&p) {}

private:
ILayout* m_layout;
IProtocol* m_protocol;
}

class SpecificDevice : public BasicDevice
{
public:
ADevice() : BasicDevice(m_layout, m_protocol) {}

private:
SomeLayout m_layout;
SomeProtocol m_protocol;
};

You can also allocate the layout & protocol on the heap, if BasicDevice
handles deallocation.
 
J

Jerry Coffin

Isn't that a bit of a far-fetched solution for something which is
exactly what inheritance was invented for?

He wants classes with common functionality, except for the
constructor, which is specialized for each class. That's exactly what
inheritance is for.

An inheritance-based solution is exactly what I was thinking of when
I said:

It doesn't sound like you need the other possibility, which
would be that you don't know the combination of protocol
and layout until run time. [ ... ]

In a case like that, you'd have a hierarchy of layouts and a
hierarchy of protocols, and the ctor for your device class would take
a reference (or pointer) to a layout and a protocol. You'd then store
those pointers/references in the device class.

This introduces some extra complexity (e.g. managing the lifetimes of
the protocol and layout objects) and run-time overhead (all the
functions in the layout/protocol that are used from the device need
to be virtual). Neither of these is huge, and both are quite
manageable. If you needed (or had some use for) the capabilities you
get from them, they'd be entirely worthwhile and reasonable. Given
that (at least according to the OP) the idea in this case was purely
to minimize duplication in the source code, _they_ are the solution
I'd call far-fetched.

There is an inheritance-based solution that eliminates most of those
problems as well. In fact, I'd consider it quite similar to the
template-based one I posted previously. It would be something like:

class ALayout {};
class BLayout {};

class AProtocol {};
class BProtocol {};

class ADevice : ALayout, AProtocol {};
class BDevice : BLayout, BProtocol {};

For those to whom that might look a bit unusual, that's multiple,
private inheritance. It does much the same as the template-based
solution: it embeds an instance of a protocol and a layout in each
device class. It has many of the same strengths and weaknesses as the
template-based version as well: each combination of protocol and
layout used to implement a device must be specified at compile time,
not at run time. There is no need for the functions to be virtual,
because we're never invoking them via a pointer/reference to the
base. Lifetime issues are trivial -- each protocol and layout pair is
created/destroyed along with the device class (i.e., just like any
other base class).

During the time after C++ got MI, but before it got templates, I
probably would have recommended this solution. Today, however, I see
little or no advantage to it over the template-based one I posted
originally.
 
J

James Kanze

far-fetched? The request mapped perfectly to what templates
do, and Jerry showed the code that gives the perfect solution
(to what was stated).
Inheritance was invented for a different thing, and would
solve this case in a contorted way if at all -- and OP seem to
be familiar with it enough, and turned here discovering the
redundancy it leaves.

I wouldn't say that inheritance was invented for this, but using
inheritance to provide "custom" initialization is a more or less
standard C++ idiom, and hardly what one would consider
contorted. (You might start by looking at something like
std::fstream, for example.)

Templates are an alternative solution. They have the
disadvantage that they can easily lead to code bloat, and that
to be effective, the initializations have to share a common
pattern; inheritance gives you a lot more freedom. On the other
hand, to use inheritance, you have to make everything needed by
the derived classes available somehow: either protected or via
an accessor function. There's no universally correct solution,
but on the whole, I find that inheritance provides the simpler
solution most of the time.
Care to show an example?

class Device
{
Protocol* my_protocol;
Layout* my_layout;

public:
Device( Protocol* desired_protocol,
Layout* desired_layout )
: my_protocol( desired_protocol )
, my_layout( desired_layout )
{
}
// ...
};

class SomeSpecificDevice
: private SpecificProtocol // inheritance to control
, private SpecificLayout // order of initialization.
, public Device
{
public:
SomeSpecificDevice() : Device( this, this ) {}
};
And in current C++ you can not inherit constructors, so how
that would be handled?

See above. It's a fairly standard idiom.

Of course, you can easily combine the two, using a template for
the derived class (parametrized on the protocol and the layout).
The derivation still gives you the liberty of handling special
cases differently, however.
The classes' "common functionality" is really common just
textually -- it invokes different functions on objects of
different types.

The original code looked to me more of something along the lines
of the strategy pattern. Where inheritance plays an important
role in the stragety classes.
Inheritance could deal with the "different function" thing
calling a virtual, but is hosed with the different member.
Certainly the class could have parts that are

Looks like something got deleted here.
 
B

Balog Pal

James Kanze said:
I wouldn't say that inheritance was invented for this, but using
inheritance to provide "custom" initialization is a more or less
standard C++ idiom, and hardly what one would consider
contorted.

Well, "contorted" may be the wrong word, I mean it leads to way more work
and way more complexity, if actually chosen.
Templates are an alternative solution. They have the
disadvantage that they can easily lead to code bloat, and that
to be effective, the initializations have to share a common
pattern; inheritance gives you a lot more freedom. On the other
hand, to use inheritance, you have to make everything needed by
the derived classes available somehow: either protected or via
an accessor function.

Exactly. In the described case it needs converting the concrete classes to
interfaces.
There's no universally correct solution,
but on the whole, I find that inheritance provides the simpler
solution most of the time.

Solution to what requirements? If you need runtime polymorphism, sure, you
go that way. If compile-time stuff is good, then it is a drag, and
disadvantage.
class Device
{
Protocol* my_protocol;
Layout* my_layout;

In the original case there was ProtocolA, ProtocolB, no generic Protocol was
mentioned usable that way. Certainly it is possible to design the syste
that way up front, or convert like that, but would you do it unless
necessary?
class SomeSpecificDevice
: private SpecificProtocol // inheritance to control
, private SpecificLayout // order of initialization.
, public Device
{
public:
SomeSpecificDevice() : Device( this, this ) {}
};

With what you're back to 'repeated code' the original question was
concerned. To make it good you go ahead to make that class a template, and
use like in Jerry's #1 variant.

But then is there a real point to do the split, and fidding with the base
instead of the direct approach? You certainly still may want a base
class, if there are parts independent of the variable elements.

the original mentioned just simple calls that is hardly code bloat -- if
that is a real concern (and the template visibly has fat functions that are
mostly indepentent, a few virtuals may be defined and the bulk moved to the
base.

But that adds complexity, so only if there is a good reason.
See above. It's a fairly standard idiom.

I'm certainly aware of this way, and used it too, thought something
different is meant.
Of course, you can easily combine the two, using a template for
the derived class (parametrized on the protocol and the layout).
The derivation still gives you the liberty of handling special
cases differently, however.
Sure.


The original code looked to me more of something along the lines
of the strategy pattern. Where inheritance plays an important
role in the stragety classes.

IMO strategy, as defined in the original form is mainly used for
runtime-changing situations. Especially in C++, where the static cases are
handled better using 'policy' or 'traits'. To me strategy implies I want
to assembe and possibly change it at runtime.
 
K

Kaz Kylheku

Hi,

I have the following problem. I have an interface (abstract base class)
to represent a "device":

class IDevice {
virtual Read () = 0;
}

I have a number of concrete devices, that implement this interface. The
typical implementation of a device consist of a protocol part (defines
how to read the data) and a layout part (defines the structure of the
data and thus where to read the data):

class Device : IDevice {
m_protocol;
m_layout;
...
}

Device::Device ()
: m_protocol (...), m_layout (...)
{
...
}

Device::Read ()
{
return m_protocol->Read (m_layout->offset);
}

But now I want to implement some devices which are very similar. For
instance two devices sharing the same layout, but using a different
protocol. What is the preferred way to implement this, without
duplicating all code?

Use aggregation for the protocol and layout rather than composition.

It almost looks like your Device class is just an empty facade that
delegates its operations to the m_protocol and m_layout that it
contains.

These members can just be references (to base classes which have
virtual functions).

The IDevice interface binds to Device, which proxies all of the calls to
these other objects. A Device instance can appear to be of effectively
different types because it can have references to different types of
protocols and layouts.
Basically, only the constructor is different. All other member functions
(such as Read) have exactly the same implementation.

ADevice::ADevice ()
: m_protocol (new AProtocol (...)), m_layout (new Layout (...))
{
...
}

BDevice::BDevice ()
: m_protocol (new BProtocol (...)), m_layout (new Layout (...))
{
...
}

So they already are pointers? You are one step away:

Device::Device(ProtocolBase *proto, LayoutBase *layout)
: m_protocol(proto)
, m_layout(layout)
{
}

Now it's up to whoever is constructing this object to configure it
with an appropriate combination of protocol and layout.

This is where the class factory pattern comes in handy.

// derives from IDeviceFactory
// makes devices configured with AProtocol and ALayout

static ADeviceFactory adf;

// factoryPtr points to some IDeviceFactory implementation;
// just call the Create virtual function to make a factory of
// its kind.

Device *dev = factoryPtr->Create();
 
J

Jerry Coffin

[ ... ]
Templates are not an option in my case for two reasons:

First, the constructor for different protocol/layout objects can be
different (e.g. different number of arguments, different types, etc),
and the code in the constructor is also not the same for each device.

Second, my actual implementation is not in C++, but in plain C (for
portability reasons). I do have an object oriented framework where
inheritance and polymorphism is done with structs and manual vtable's,
but templates is simply not possible. I posted to a C++ newsgroup
because I design in C++ and then port the concept to my C environment.
It's easier to visualize the concept without the C oop boilerplate code.

Yup, under those circumstances, it sounds like inheritance is nearly
the only possibility.
 
J

Jef Driesen

Am 09.01.2010 20:14, schrieb Jef Driesen:

Read about Dependency injection, i.e.:
http://en.wikipedia.org/wiki/Dependency_injection

In short, you pass a pointer/reference of a specific service
implementation to a class, so that the class doesn't have to create it,
and doesn't even have to know about it's existance (only the abstract base).

class BasicDevice : public IDevice
{
public:
virtual Read() { /* ... */ }; // common implementation

protected:
// this class can't be instantiated directly
BasicDevice(layout& l, protocol& p)
: m_layout(&l), m_protocol(&p) {}

private:
ILayout* m_layout;
IProtocol* m_protocol;
}

class SpecificDevice : public BasicDevice
{
public:
ADevice() : BasicDevice(m_layout, m_protocol) {}

private:
SomeLayout m_layout;
SomeProtocol m_protocol;
};

You can also allocate the layout& protocol on the heap, if BasicDevice
handles deallocation.

This looks like a very interesting idea!
 
J

Jef Driesen

Use aggregation for the protocol and layout rather than composition.

It almost looks like your Device class is just an empty facade that
delegates its operations to the m_protocol and m_layout that it
contains.

The real code is usually a little more complex. The one line example was
just to illustrate that the common code needs access to the protocol and
layout to do its job.
These members can just be references (to base classes which have
virtual functions).

The IDevice interface binds to Device, which proxies all of the calls to
these other objects. A Device instance can appear to be of effectively
different types because it can have references to different types of
protocols and layouts.


So they already are pointers? You are one step away:

Device::Device(ProtocolBase *proto, LayoutBase *layout)
: m_protocol(proto)
, m_layout(layout)
{
}

They are not necessary pointers, but they could be if that would be
required for a particular solution. I didn't specify the type of the
m_protocol/layout members on purpose, to not rule out any idea from the
start. The use of new in the constructor was just to illustrate that for
an XDevice, a XProtocol is needed.
Now it's up to whoever is constructing this object to configure it
with an appropriate combination of protocol and layout.

This is where the class factory pattern comes in handy.

// derives from IDeviceFactory
// makes devices configured with AProtocol and ALayout

static ADeviceFactory adf;

// factoryPtr points to some IDeviceFactory implementation;
// just call the Create virtual function to make a factory of
// its kind.

Device *dev = factoryPtr->Create();

This appears to be very similar to the named constructor idiom [1], that
I have been looking at. Except that these named constructors are now
hidden behind the factory.

[1] http://www.parashift.com/c++-faq-lite/ctors.html#faq-10.8
 

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

No members online now.

Forum statistics

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

Latest Threads

Top