Design question - implentation and composition

L

Lionel

Hmmm, the word for implementing an interface has slipped my mind, anyway.

I have a design where it seems appropriate to use a tactic I haven't
seen before.

I've got four classes each of which subclass a common super class. There
are four interfaces which are divided into two groups. Each of the sub
classes must implement one interface from each group. However this has
meant that I have had some code duplication. This duplication is only
simple, a couple of setters and getters, at most any method is 4 lines long.

However, it seems appropriate to reuse the code because it /is/
identical and always should be. So the thought comes to mind that I
implement each of the interfaces with a separate class. I then contain
an instance of the two required implementations in each of the four
subclasses, while also still implement the two interfaces and the
methods just become wrappers around the instances of the implementations
of the interfaces.

The question:
Is this a strange design? Is there a better way to do it?

In fact the design is more complicated, it implements the abstract
factory pattern providing two factory methods. There is also a subclass
of each of the four classes which provides additional functionality
again, each subclass implementing a new interface.

Thanks

Lionel.
 
D

Daniel Pitts

Lionel said:
Hmmm, the word for implementing an interface has slipped my mind, anyway.

I have a design where it seems appropriate to use a tactic I haven't
seen before.

I've got four classes each of which subclass a common super class. There
are four interfaces which are divided into two groups. Each of the sub
classes must implement one interface from each group. However this has
meant that I have had some code duplication. This duplication is only
simple, a couple of setters and getters, at most any method is 4 lines long.

However, it seems appropriate to reuse the code because it /is/
identical and always should be. So the thought comes to mind that I
implement each of the interfaces with a separate class. I then contain
an instance of the two required implementations in each of the four
subclasses, while also still implement the two interfaces and the
methods just become wrappers around the instances of the implementations
of the interfaces.

The question:
Is this a strange design? Is there a better way to do it?

In fact the design is more complicated, it implements the abstract
factory pattern providing two factory methods. There is also a subclass
of each of the four classes which provides additional functionality
again, each subclass implementing a new interface.

Thanks

Lionel.

I might first try to simplify by reducing the number of interface
needed. Then, once I've done that, re-evaluate the heirarchy. Its hard
to tell you if the design is good without more details about the
interfaces/classes.
 
L

Lionel

Daniel said:
I might first try to simplify by reducing the number of interface
needed.

The interfaces are required in my opinion. There is a lot of common
functionality in the super class, the majority of the code resides here,
however, each subclass must implement these interfaces, or somehow
contain the methods that are in these interfaces. I don't see how there
is a way around that.

Then, once I've done that, re-evaluate the heirarchy.

I've considered the heirarchy a number of times and considered what
other options I could use. I keep seeing the current heirarchy as the best.

Its hard
to tell you if the design is good without more details about the
interfaces/classes.


Understandably. It is hard to give further detail without information
overload. I might think about posting the UML class diagrams somewhere.

Thanks

Lionel.
 
D

Daniel Pitts

Lionel said:
The interfaces are required in my opinion. There is a lot of common
functionality in the super class, the majority of the code resides here,
however, each subclass must implement these interfaces, or somehow
contain the methods that are in these interfaces. I don't see how there
is a way around that.


I've considered the heirarchy a number of times and considered what
other options I could use. I keep seeing the current heirarchy as the best.

Its hard


Understandably. It is hard to give further detail without information
overload. I might think about posting the UML class diagrams somewhere.

Thanks

Lionel.

Or, give simplified examples.
Is there a reason that the interfaces can't be combined? Is there a
reason that the base class can't implement the interfaces? etc... and
so forth.
 
C

Chris Uppal

Lionel said:
Hmmm, the word for implementing an interface has slipped my mind, anyway.

"Implement" ;-)

I've got four classes each of which subclass a common super class. There
are four interfaces which are divided into two groups. Each of the sub
classes must implement one interface from each group.

My knee-jerk reaction is that this is probably over-complicated. But then, I
don't know what forces led you to this design.

However this has
meant that I have had some code duplication. This duplication is only
simple, a couple of setters and getters, at most any method is 4 lines
long.

Getters and setters in an interface ? Indeed, getters and setters are highly
suspect in any OO design.

However, it seems appropriate to reuse the code because it /is/
identical and always should be.

A reasonable reason ;-)

So the thought comes to mind that I
implement each of the interfaces with a separate class. I then contain
an instance of the two required implementations in each of the four
subclasses, while also still implement the two interfaces and the
methods just become wrappers around the instances of the implementations
of the interfaces.

I think the important point here is that -- in a sense -- it doesn't matter.
As long as these details are not externally visible (which includes: not
constraining the design of the super- or sub-classes) then you can implement
those "duplicated" methods in any way you want. If it feels right to pull the
duplication out into a separate class, then go for it. If it feels like
over-engineering then don't.

BTW, another option (which may make no sense at all in your context) would be
to put the shared methods (with 'protected' visibility) into the common
superclass, and then re-expose them only in the subclasses where they make
sense. Of course, that means that the super-class is "aware" of its subclasses
in the sense that it has been co-designed with them, and has features that are
specifically intended for those subclasses. But I don't think there is
anything wrong with such clusters of co-designed classes, as long as you
realise that the sub-class/super-class relationship is not the same as in
arbitrary inheritance.

In fact the design is more complicated, it implements the abstract
factory pattern providing two factory methods. There is also a subclass
of each of the four classes which provides additional functionality
again, each subclass implementing a new interface.

See the above knee-jerk reaction. This complexity of inheritance, while not
obviously /wrong/, does suggest that you may be over-using subclassing. It
hints that you may be using sub-classing where containment would do the job
better -- and indeed your proposed design is moving towards containment in a
small way. Maybe (just maybe) the uneasiness you feel is because you haven't
gone far enough in that direction.

It's probably worth thinking about what would happen to your design if you
removed the shared superclass altogether, and moved the common stuff into one
or more helper objects which are uses /by/ your concrete classes (a
generalisation of what you are already considering).

-- chris
 
L

Lionel

Daniel said:
Or, give simplified examples.

I'll work on a simplified example.

Is there a reason that the interfaces can't be combined?

Briefly, if we have interfaces InterfaceA1, InterfaceB1, InterfaceA2 and
InterfaceB2.

We must have four classes as follows:

Class1 implements InterfaceA1, InterfaceB1

Class2 implements InterfaceA1, InterfaceB2

Class3 implements InterfaceA2, InterfaceB1

Class4 implements InterfaceA2, InterfaceB2

Each class produces quite different results (drug dose optimisation).

Is there a
reason that the base class can't implement the interfaces? etc... and
so forth.

There are several methods in the base class that are common to all
subclasses. A lot are implemented in the base class and there are some
abstract ones that the base class depends on implementation in subclass.

This probably doesn't clarify too much so I will work on an example and
post back in a new thread in the near future.

Thanks for you help :).

Lionel.
 
L

Lionel

Chris said:
It's probably worth thinking about what would happen to your design if you
removed the shared superclass altogether, and moved the common stuff into one
or more helper objects which are uses /by/ your concrete classes (a
generalisation of what you are already considering).

I will continue to think about this.

Lets say I have the following classes:

SuperClass

SubClass1 implements InterfaceA1, InterfaceB1

SubClass2 implements InterfaceA1, InterfaceB2

SubClass3 implements InterfaceA2, InterfaceB1

SubClass4 implements InterfaceA2, InterfaceB2

And of course the corresponding interfaces.

There is only one class that ever even knows about any of the
sub-classes, that's the class which acts as an interface between the
database and the rest of the application.

As for the rest of the application it relies on a few regular methods in
and a few abstract methods in SuperClass for determining all the
information required for performing all operations. There are three
factory methods which construct instances of another superclass which
again provides a similar hierarchy as above.

The reason for using this hierarchy is so that I never have to worry
about the exact implementation in the rest of the application, it gets
figured out once when retrieving and saving info from/to the database
and that's it.

There was someone else working on the code for a period when I was away
and they hadn't designed using any polymorphism. The result was
incredibly ugly, there were 3-4 massive switch statements that did
pretty much the same thing - errrr, I still shudder at how long it took
me to figure out what was going on!

I said in another branch of this thread that I would work on a
simplified example. Soon I'm going to be able to provide access to the
source which would free me up a bit.

Background info: It's an open source GPL application for antibiotic dose
optimisation. We are developing it at the University of Queensland in
Australia. I'm the developer, there are two pharmacists that I work with
who provide all the domain knowledge.

Lionel.
 
D

Daniel Pitts

Lionel said:
I will continue to think about this.

Lets say I have the following classes:

SuperClass

SubClass1 implements InterfaceA1, InterfaceB1

SubClass2 implements InterfaceA1, InterfaceB2

SubClass3 implements InterfaceA2, InterfaceB1

SubClass4 implements InterfaceA2, InterfaceB2

And of course the corresponding interfaces.
So, the implementation of Interface B1 in Subclass1 is
similar/different from the implementation in SubClass3?

If it is similar, then maybe you should have instead A1impl, B1impl,
A2impl B2impl.
Then, SuperClass would have a constructor(A, B)
Although, if there are combinations of options, maybe they don't realy
share a common base class, and you shouldn't use interfaces this way.

Perhaps a small snippet of the interfaces would help us help you.
 
L

Lionel

Quick summary of the below. There is too much to explain, I've only
really touch the simplest part. I won't be offended if you look at it
and choose not to respond because it is difficult to provide everything
necessary.


Daniel said:
So, the implementation of Interface B1 in Subclass1 is
similar/different from the implementation in SubClass3?

They are very similar. Which is why I'm asking the question :).
If it is similar, then maybe you should have instead A1impl, B1impl,
A2impl B2impl.

I'm with you on that.
Then, SuperClass would have a constructor(A, B)

But here I think it falls down. A1 is more like the opposite of A2 so
this design would mean that SuperClass would need some concept of who
and what A and B are and what they do. I need to think about this more
because you may have raised a possibility that I need to consider.
Although, if there are combinations of options, maybe they don't realy
share a common base class, and you shouldn't use interfaces this way.

Perhaps I can dispose of the interfaces. Let's take SubClass1 and
SubClass2 that both implement InterfaceA1. The main time that this is
useful is when I'm saving the data from the subclasses or when I'm
creating new instances of the subclass from data from the database:

public static void saveClass(SuperClass superClass) {
//save the common data
...

//look at what else needs saving
if(superClass instanceof InterfaceA1) {
saveInterfaceA1((InterfaceA1)superClass);
}
if(superClass instanceof InterfaceA2) {
saveInterfaceA2((InterfaceA2)superClass);
}
//and so on
}

private static void saveInterfaceA1(InterfaceA1 a1) {
//construct sql query and save data such as a1.getInterfaceInt()
}

Similarly when we are reading data from the database if we have an
instance of SubClass1 then we know there is a table with an entry
containing the data that can be set using the setters in InterfaceA1.

It's convenient to know that an implementation of InterfaceA1 contains
certain methods.

Perhaps a small snippet of the interfaces would help us help you.


You are right. I'm going to have to figure out how I can give you more
data before we continue. The problem is larger than what we have
discussed so far and I need to portray the information without posting a
ridiculous amount otherwise I am wasting your time.

A brief summary is that each interface is very simple, four or so
methods. Say you have a drug, the input method can be intravascular or
extravascular. One interface is IVDrug and contains setters and getters
for minInfusionTime and maxInfustionTime stating the min and max time to
give the drug into the vein. EVDrug has a rate constant Ka which
describes how quickly the drug goes from tablet form in the stomach into
the bloody. Ka can be expressed as a formula. There is also a
variability on Ka, so it is accurate +- some percentage amount.

e.g.

interface IVDrug {
public int getMinInfusionTime();
public int getMaxInfusionTime();
}

interface EVDrug {
public String getKaEquation();
public double getKaVariability();
}

I'll continue:

interface TwoCompDrug {
public String getVpEquation();
public double getVpVariability();
public String getQEquation();
public String getQVariability();
}

Actually the 4th interface doesn't exist, it would however be
OneCompDrug but there is nothing special about it.

A subclass is either (One Compartment or Two Compartment) AND
(intravascular or extravascular).



Overall my feeling is that the code is fairly nice. It's easy to read
and understand and recent extensions were very simple to design and
implement. My view is that even if this isn't the best design it is
still highly readable. There is some slight duplication of code.

It's just in my best efforts to continue improving my design skills that
I seek to see if there is a better approach. I haven't had the luxury of
being taught this stuff. University courses introduce you to software
patterns, OO design etc. but when it comes to practice it is a different
story. I just want to make sure I do the best job possible.

Thanks for your comments

Lionel.
 
D

Daniel Pitts

Lionel said:
Quick summary of the below. There is too much to explain, I've only
really touch the simplest part. I won't be offended if you look at it
and choose not to respond because it is difficult to provide everything
necessary.




I'm with you on that.


But here I think it falls down. A1 is more like the opposite of A2 so
this design would mean that SuperClass would need some concept of who
and what A and B are and what they do. I need to think about this more
because you may have raised a possibility that I need to consider.


Perhaps I can dispose of the interfaces. Let's take SubClass1 and
SubClass2 that both implement InterfaceA1. The main time that this is
useful is when I'm saving the data from the subclasses or when I'm
creating new instances of the subclass from data from the database:

public static void saveClass(SuperClass superClass) {
//save the common data
...

//look at what else needs saving
if(superClass instanceof InterfaceA1) {
saveInterfaceA1((InterfaceA1)superClass);
}
if(superClass instanceof InterfaceA2) {
saveInterfaceA2((InterfaceA2)superClass);
}
//and so on
}

private static void saveInterfaceA1(InterfaceA1 a1) {
//construct sql query and save data such as a1.getInterfaceInt()
}

Similarly when we are reading data from the database if we have an
instance of SubClass1 then we know there is a table with an entry
containing the data that can be set using the setters in InterfaceA1.

It's convenient to know that an implementation of InterfaceA1 contains
certain methods.




You are right. I'm going to have to figure out how I can give you more
data before we continue. The problem is larger than what we have
discussed so far and I need to portray the information without posting a
ridiculous amount otherwise I am wasting your time.

A brief summary is that each interface is very simple, four or so
methods. Say you have a drug, the input method can be intravascular or
extravascular. One interface is IVDrug and contains setters and getters
for minInfusionTime and maxInfustionTime stating the min and max time to
give the drug into the vein. EVDrug has a rate constant Ka which
describes how quickly the drug goes from tablet form in the stomach into
the bloody. Ka can be expressed as a formula. There is also a
variability on Ka, so it is accurate +- some percentage amount.

e.g.

interface IVDrug {
public int getMinInfusionTime();
public int getMaxInfusionTime();
}

interface EVDrug {
public String getKaEquation();
public double getKaVariability();
}

I'll continue:

interface TwoCompDrug {
public String getVpEquation();
public double getVpVariability();
public String getQEquation();
public String getQVariability();
}

Actually the 4th interface doesn't exist, it would however be
OneCompDrug but there is nothing special about it.

A subclass is either (One Compartment or Two Compartment) AND
(intravascular or extravascular).



Overall my feeling is that the code is fairly nice. It's easy to read
and understand and recent extensions were very simple to design and
implement. My view is that even if this isn't the best design it is
still highly readable. There is some slight duplication of code.

It's just in my best efforts to continue improving my design skills that
I seek to see if there is a better approach. I haven't had the luxury of
being taught this stuff. University courses introduce you to software
patterns, OO design etc. but when it comes to practice it is a different
story. I just want to make sure I do the best job possible.

Thanks for your comments

Lionel.

So, in the implementations of the methods you listed above, do those
calculate a value, or are they simply accessors to a field?
If they both are only simple accessors, I would suggest changing the
"is a" into a "has a", and simple have concrete objects.
I think that our questions here may have been focused on the wrong part
of your program... Do you have a "instanceof" checks in your code? If
so, then it sounds like you might need to rethink your (lack of) use of
polymorphism. Interfaces should usually describe a set of actions on
(behavoir of) an object.

I'd be interested in seeing the big picture, and making suggestions
from there. I would suggest reading "Refactoring: Improving the Design
of Existing Code" (Fowler, Beck, Brant, Opdyke, Roberts)
http://www.amazon.com/Refactoring-Improving-Design-Existing-Code/dp/0201485672

Look up "Refactoring: Replace Type Code with State/Strategy" or
"Refactoring: Replace Type Code with Subclasses"
It sounds to me like you have two different concepts that should be
seperated from the heirarcy altogether.
 
L

Lionel

Daniel said:
So, in the implementations of the methods you listed above, do those
calculate a value, or are they simply accessors to a field?

Accessors in the classes we've been talking about.
If they both are only simple accessors, I would suggest changing the
"is a" into a "has a", and simple have concrete objects.

Ok, that makes sense and is simple. I'm just curious. This means that
the base class must know about its subclasses and implementations. If
for some reason in the future some pharmacist figured out that some new
model was appropriate, they would have to change the base class to tell
it about additional fields/accessors.
Do you have a "instanceof" checks in your code?

Once when saving the class to the database. Never again after that.
If
so, then it sounds like you might need to rethink your (lack of) use of
polymorphism.

The specifics of what we are talking about there is no polymorphism, but
amongst other polymorphic methods there are a bunch of abstract methods
in the base class that must be implemented in subclasses. As I said,
I've used the Abstract Factory pattern, there are three factory methods,
we don't need to go into that though, I'm confident with that component
as I've used a known design pattern. There is only the one instance
where I have to worry about what the subclass type is and that is in the
class that interfaces to the database.

I'd be interested in seeing the big picture, and making suggestions
from there.

Is your email address is correct then I will notify you when I come up
with a more complete picture.
I would suggest reading "Refactoring: Improving the Design
of Existing Code" (Fowler, Beck, Brant, Opdyke, Roberts)
http://www.amazon.com/Refactoring-Improving-Design-Existing-Code/dp/0201485672

Great, I got myself a copy and will have a read.

Lionel.
 
D

Daniel Pitts

Lionel said:
Accessors in the classes we've been talking about.


Ok, that makes sense and is simple. I'm just curious. This means that
the base class must know about its subclasses and implementations. If
for some reason in the future some pharmacist figured out that some new
model was appropriate, they would have to change the base class to tell
it about additional fields/accessors.


Once when saving the class to the database. Never again after that.


The specifics of what we are talking about there is no polymorphism, but
amongst other polymorphic methods there are a bunch of abstract methods
in the base class that must be implemented in subclasses. As I said,
I've used the Abstract Factory pattern, there are three factory methods,
we don't need to go into that though, I'm confident with that component
as I've used a known design pattern. There is only the one instance
where I have to worry about what the subclass type is and that is in the
class that interfaces to the database.



Is your email address is correct then I will notify you when I come up
with a more complete picture.


Great, I got myself a copy and will have a read.

Lionel.

If you send a message to my e-mail, while technically correct, will
probably result in your message going straight to the junk mail folder.
Try sending to Lionel@, instead of googlegroupie@. Hopefully I'll
remember you :) You can also leave a message anywhere on
http://forums.virtualinfinity.net/
 
C

Chris Uppal

Lionel said:
Perhaps I can dispose of the interfaces. Let's take SubClass1 and
SubClass2 that both implement InterfaceA1. The main time that this is
useful is when I'm saving the data from the subclasses or when I'm
creating new instances of the subclass from data from the database:

public static void saveClass(SuperClass superClass) {
//save the common data
...

//look at what else needs saving
if(superClass instanceof InterfaceA1) {
saveInterfaceA1((InterfaceA1)superClass);
}
if(superClass instanceof InterfaceA2) {
saveInterfaceA2((InterfaceA2)superClass);
}
//and so on
}

It's starting to sound as if you are using the interfaces as something very
like metadata -- it tells things (mostly the DB code) what "features" each drug
has. If that's the case then maybe you would find it better to make that
concept explicit, so that you can ask each drug for its list of features, and
then ask the features to save themselves to the DB.

By "explicit" I mean that you represent the feature at the OO level, as real
objects with real behaviour, rather than by hacking around with inheritance.

BTW, I find it rather odd that something outside the object is responsible for
writing it to the DB -- surely the object itself is best placed to know what
should be written ?

-- chris
 
L

Lionel

Chris Uppal wrote:
[snip] comments here noted and considered.
BTW, I find it rather odd that something outside the object is responsible for
writing it to the DB -- surely the object itself is best placed to know what
should be written ?

If every object had to know about the database and how to construct SQL
queries and save themselves the code would become a mess. The solution I
use makes a very nice interface in which one class is responsible for
the database whether that be MySQL, Access, PostGreSQL etc.

This is common practice AFAIK.

Lionel.
 
T

Tom Forsmo

Hi

It sound to me that you are trying to model a class hierarchy that is
designed around variant properties. I think it would be better to use
composition instead. I don't quite understand all of your design yet,
but as an example lets take the DB saveClass functionality.

By using composition and interfaces you could for example do the
following. Lets say you have a class named Drug which is composed of a
list of drug properties. The drug properties list would hold objects
that implements a saveToDB() method. So, to save the list of properties
to a db you would only need to iterate the list and call the saveToDB().
Here is a code example that only considers the db storage aspect.


interface DbStorage {
public int saveToDb(Connection dbConn);
}

public class DrugPropertyA implements DbStorage, PropertyA {

public int saveToDb(Connection dbConn) {
// details on how to save a DrugProperty for the
// property A class type.
}
}

public class DrugPropertyB implements DbStorage, PropertyB {

public int saveToDb(Connection dbConn) {
// details on how to save a DrugProperty for the
// property B class type
}

}

public class Drugs {

List<DbStorage> prop = new ArrayList();

public List<DbStorage> getDrugProperties() {
return prop;
}
}

public class DbManager {

public int storeDrugs(List<DbSTorage>) {

for(DbStorage e : prop) {
e.saveToDb(dbcon);
}
}
}

This way you don't have to consider the type of the properties. I am not
saying this works for your design, but I think you should be looking in
that direction for a solution.

You could extend this by doing

public class Drug {

protected String PropertyName;
....
}

public class DrugA extends DrugProperty implements DbStorage, PropertyA {
...
}

But I suspect that's what you already have done.


I am assuming what you are talking about above is actually the specific
example below, right?
interface IVDrug {
public int getMinInfusionTime();
public int getMaxInfusionTime();
}

interface EVDrug {
public String getKaEquation();
public double getKaVariability();
}

I'll continue:

interface TwoCompDrug {
public String getVpEquation();
public double getVpVariability();
public String getQEquation();
public String getQVariability();
}

I am not sure of exactly how to do this, because the problem is that you
have different methods for different drugs. But my question is: could
you express this in a different way? My thought is that composition is
the way to go here as well, but in a different way. For example, at some
point you need to know what specific methods you need to call to get
what you want. Could the properties be expressed by a hash instead,
which only contained what was relevant for the current drug? Its not
very OO I suppose, but I think its a better way. Or if how you use these
methods is done i such a way that you could use an exec()/calc() method
in each drug/property instead, which returned the final result.
Or providing the necessary arguments to the method?

E.g.

public interface PropertyCalculation {

public float calc(int amount, int something);
}

public class PropertyA implements PropertyCalculation {

public float calc(int amount, int something) {

return (vpEquation * amount / something) / qeEquation /
vpVariability);
}
}

public class Drug {

calculateX() {
for(PropertyCalculation c : prop) {
xValue += e.exec();
}
}
}

This was just an example, which might not even be relevant, but its
difficult when not having all the details.

tom
 

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,743
Messages
2,569,478
Members
44,898
Latest member
BlairH7607

Latest Threads

Top