Edit 2: TL;DR: Is there a way not to break OO best practices while still satisfying the constraint that a bunch of things of the same kind have to be convertible to a canonical thing of that kind?
Also, please keep in mind that my question is about the general situation, not the specific example. This isn't a homework problem.
Suppose you have the following:
- an abstract base class that implements common functionality;
- a concrete derived class that serves as the canonical representation.
Now suppose you want any inheritor of the base class to be convertible to the canonical representation. One way of doing this is by having an abstract method in the base class that is meant to return a conversion of the inheritor as an instance of the canonical derived class.
However, it seems to be generally accepted that base classes should not know about any of their derived classes, and in the general case, I agree. However, in this scenario, this seems to be the best solution because it enables any number of derived classes, each with their own implementation we don't need to know anything about, to be interoperable via the conversion to the canonical representation that every derived class has to implement.
Would you do it differently? Why and how?
An example for geometric points:
// an abstract point has no coordinate system information, so the values
// of X and Y are meaningless
public abstract class AbstractPoint {
public int X;
public int Y;
public abstract ScreenPoint ToScreenPoint();
}
// a point in the predefined screen coordinate system; the meaning of X
// and Y is known
public class ScreenPoint : AbstractPoint {
public ScreenPoint(int x, int y) {
X = x;
Y = y;
}
public override ScreenPoint ToScreenPoint()
=> new ScreenPoint(X, Y);
}
// there can be any number of classes like this; we don't know anything
// about their coordinate systems and we don't care as long as we can
// convert them to `ScreenPoint`s
public class ArbitraryPoint : AbstractPoint {
private int arbitraryTransformation;
public ArbitraryPoint(int x, int y) {
X = x;
Y = y;
}
public override ScreenPoint ToScreenPoint()
=> new ScreenPoint(X * arbitraryTransformation, Y * arbitraryTransformation);
// (other code)
}
Edit 1: The reason AbstractPoint
and ScreenPoint
are not the same class is semantic. An AbstractPoint
does not have a defined coordinate system, therefore the values of X and Y in an AbstractPoint
instance are meaningless. A ScreenPoint
does have a defined coordinate system, therefore the values of X and Y in a ScreenPoint
instance have a well-defined meaning.
If ScreenPoint
were the base class, then an ArbitraryPoint
would be a ScreenPoint
, which is not the case. An ArbitraryPoint
can be converted to a ScreenPoint
, but that does not mean that it is-a
ScreenPoint
.
If you are still unconvinced, consider that an arbitrary coordinate system ACS1
can be defined as having a dynamic offset to the screen coordinate system SCS
. This means the mapping between the two coordinate systems can vary with time, i.e. the point ACS1 (1, 1)
can map to SCS (10, 10)
at one moment, and SCS (42, 877)
at another moment.
You have complicated things without reason. If a class cannot implement the specification of the base class you are violating Liskovs Substitution principle.
An alternative is an interface that declares that an arbitrary object can be represented as coordinates:
Then it doesn't matter what the object is or what its internal handling of coordinates is.
Also do not forget that inheritance is about
is-a
relationship. If any of the sub classes do not support what the base class provide, it's not really ais-a
relationship. That typically indicates that the base class really is a utility class or that the defined hierarchy isn't well designed.This kind of design is usually a code smell. Base classes should not know about their derived classes because it creates a circular dependency. And circular dependencies usually lead to complicated designs where it's difficult to reason about what classes should be doing. In Java, base classes knowing about their derived classes can even result in deadlock in some rare cases (I don't know about C#).
However, you can break general rules in special cases, when you know exactly what you are doing, specially if what you are trying to achieve is simple enough.
And your case here seems to be simple enough. Having
AbstractPoint
andScreenPoint
as different classes is correct. But in fact they "work together": AllAbstractPoint
s should be able to convert toScreenPoint
(that's maybe the most important functionality inAbstractPoint
's contract?). Since one cannot exist without the other, there is nothing wrong aboutAbstractPoint
knowing aboutScreenPoint
.Update
In a different design: Create an interface called
CanonicalPoint
.AbstractPoint
has a method calledToCanonicalPoint
, which returnsCanonicalPoint
. All derived classes ofAbstractPoint
have to implement this and returnCanonicalPoint
.ScreenPoint
is a derived class ofAbstractPoint
which implements theCanonicalPoint
interface. You could even have more than one derived class that implementsCanonicalPoint
. Note: IfAbstractPoint
andCanonicalPoint
have methods in common, both can implement another interface called, say,Pointable
, that declares all of these methods.I feel that the Single responsibility principle in SOLID has been overlooked. The
AbstractPoint
and its concrete implementationScreenPoint
, are essentially storing data, e.g.X, Y
, unique to the class group. As well the base classAbstractPoint
is trying to enforce what seems like the Factory Method pattern inline (minus an interface being returned). While I think it to be appropriate and necessary to have logical operations on the data in theAbstractPoint
and/orScreenPoint
classes; I feel like a separateScreenPointFactory
class implementing Factory Method pattern to instantiate theScreenPoint
is much needed here.If you are needing to create thousands of
ScreenPoint
classes, having the extra virtual method calls such asToScreenPoint()
and extra object size could have negative performance issues. Considering to use the Flyweight pattern in this scenario could help load times. For Flyweight pattern to be successful, you will need the Factory Method pattern to be implemented as the Flyweight pattern is used within the factory. Since there will be factories, you will need anIPoint
interface and simply deriveAbstractPoint
off of the interfaceIPoint
.IPoint
will simply holdX, Y
for now. The remaining point types gain theIPoint
interface through inheritance ofAbstractPoint
. Since there will be group of related point types, e.g.ArbitraryPoint
,ScreenPoint
, they can all be instantiated via an Abstract Factory.Programming to interfaces, not implementations principle will be important here. To pull off the functionality of
ToScreenPoint()
in what I am describing, I would removeToScreenPoint()
from the Point classes completely and instead, I would create anArbitraryTransformation
object configured upon startup. I would use Dependency Injection to place theArbitraryTransformation
object into the appropriate factory during IOC configuration. Then when the Abstract Factory methods are called to create any new variations of your screen points… they are created with the arbitrary transformation already calculated, as each independent factory method within theAbstractFactory
will use the appropriately configuredArbitraryTransformation
to perform the calculation.Doing all this will put less stress on your design, and will keep your objects a bit lighter and loosely coupled. I feel you are dealing with complexities here to where you can figure out what I just said above simply with the GoF pattern language.
However, if you or anyone would rather have the coded samples, I can come back in and provide a sample solution. Just seems like a whole lot of code to link, if you do not exactly need or want what I am suggesting.I went ahead and developed a sample solution on my GitHub called Point Example. Let me know what you think.
Also, for every design pattern added here, it will introduce extra layers of complexity to your app, so while that can be a problem in itself, I think it will lend itself to be useful for what you need.
Update
In the point example I decided to do a very simple implementation of Object Pool pattern instead of the mentioned
Flyweight pattern
above. I showed placement of a performance improvement pattern in doing so in case a solid example was needed. Although there are better ways to give the implementation, theObject Pool pattern
as used in theScreenPointFactory
is a mere functional place holder.I have updated the code in the Point Example to reflect naming things
Flyweight pattern
toObject Pool pattern
. Sorry for any confusion. Also, here are answers to Miroslav Policki's questions in the comments section below:Question 1:
1. It seems your motivation for moving onto the Factory Method pattern is the ability to use the Flyweight pattern. However, as I understand it, the Flyweight pattern is a performance optimization, and as such shouldn't be applied prematurely. So, in situations where the Flyweight pattern is unnecessary, would you still proceed with the rest of your design, and why? – Miroslav Policki
Answer: I made a correction referencing the
Object Pool pattern
instead ofFlyweight pattern
to the code. Regardless of my naming mishap, bothObject Pool pattern
andFlyweight pattern
are performance optimizations. However, my motivation for creating factories is out of habbit, as it is common to do so to encapsulate creational logic. Yes my implementation in the example code ofObject Pool pattern
was never asked for, and it is a perfomance optimization that is applied prematurely in this situation. However, I felt it to be more of an educational bonus, if somebody was wondering where to place performance enhancements in this paradigm, without spending too much time. I gaveArbitraryPointFactory
no performance enhancements to show the flexibility of the factories.Also, yes, I would absolutely use the Factories to create any variations of points, as from the given example there were mulitple concrete types. Keeping the factories around will keep instantiations of point ojbects in a single location in code, and factories will yield a single place in code to modify creation logic. Factories are useful for simple and complex objects, and allow us to program to interfaces, and not concrete implementations. However, like most design patterns, I would not suggest for anyone to have to indroduce factories into their code, if there is only one concrete point object or one place in thier code that needs to call the
new
operator for an object. However if you have two instances in your code where thenew
operator is being used to create concrete points, etc then it may be nice to have a factory so you can make a change in one place in a factory, instead of having to search through code and run into the possibilty of not updating all places appropriately if there are changes to the way the object needs to be created.Question 2:
2. You apply the transformation on a point during its creation in the corresponding factory method. However, this is different from the semantics in my example, because each
ArbitraryPoint
has internal state which can change at any time, and thus yield differentScreenPoint
s. How would you incorporate this into your design? – Miroslav PolickiAnswer: Part of me wants to say the name
ScreenPoint
has made me performance antsy, and I am striving for a relevant implementation. The other part of me wants to say I would have designed it this way regardless. However, I know you are suggestingScreenPoint
being pixel related, etc... is not the case, it is just another arbitrary example. I will give you my valid example first though. I was contemplating the creation of thousands apon thousands ofScreenPoint
renders in a video game, possibly where we may also be storing the color of each pixel per screen point, and we are just creating a full screen render of throw awayScreenPoint
objects. So lets say there is a double buffer of screen points going, where we are drawing two screen renders at a time. We would just be calculating the render of game object coordinates to real world coordinates and then mapping them to a 2D screen render 30 to 60+ times a second. In this scenario, I wouldn't care about modifying existing rendersX,Y
data, rather than just creating a new set ofScreenPoint
objects for each additional render. Now that can seem wasteful, andFlyweight pattern
could help to overcome having to create so many new objects at runtime as there would be only oneScreenPoint
object, and the implemenation of eachX,Y
would be encapsulated to an internalArray
orDictionary
. However, again I was also considering object size and how many method and virtual method pointers each object had to maintain along with the data, when creating or dealing with thousands ofScreenPoint
objects very quickly when thinking of my design. The design allowsScreenPoint
to be purely a data object, nothing more.Back to your semantics though, I achieved the calculation through object composition via the
ArbitraryTransformation
object and related interfaces. There is nothing preventing us the ability to compose theAbitraryTransformation
object with yourArbitraryPoint
object or aNoOpTransformation
with yourScreenPoint
to be utilized in object properties or mutator methods to perform the calculation from within theArbitraryPoint
orScreenPoint
objects respectively. This is whereHas-A
is better thanIs-A
comes into play with object composition. The factory is a good place to compose your objects together with other objects, e.g.Has-A
. We may later want to use aMochTransformation
object in unit tests with aMochScreenPointFactory
to be able to actually test aScreenPoint
object's functionality or math. So sticking with this paradigm allows for this to naturally happen in configuration (IOC Container buildup), keeping what may be a complex algorithm or impossible scenario to test against in it's own replaceable tranformation object. So again, Nothing is preventing you from keeping theToScreenPoint()
method on yourScreenPoint
object if you like having the interface there instead of handing off responsibility to the factory. My re-organization of where the calculation was happening was geared towards keeping theScreenPoint
simple and small. It's single responsibility was in just keeping up with data.I also am seeing that in your example you are using the
new
operator forScreenPoint
in two places, or eachToScreenPoint
method. With the appropriate injected factory into each point, the factory method could be called from theToScreenPoint()
method of each type of point. There is nothing preventing this either, and there is some other point the code will be instantiating the firstScreenPoint
orAbstractPoint
which could be delegated to a factory instead of setup in each location aScreenPoint
needs to be created. Also, I do not like how theAbstractPoint
class is returning a single concrete instance ofScreenPoint
rather than an interface. You have simply embedded the notion of anAbstract Factory pattern
for a family of objects, yet every family returns a concrete instance of one type, rather than an interface. Because of this your calling code will need to be context aware regardless. This is why I broke out in my example the notion thatScreenPoint
andArbitraryPoint
derived from the same interface and yetArbitraryPoint
had it's own extra / custom data. If I need to perform the conversion from arbitrary point to screen point, I had to be context aware anyways enough to use the correct Factory method to create a new point with the correct calculation, which in turn utilizes theArbitraryTransformation
calculation for instance at the moment of object creation.I think where my example may differ from yours in intent, is that you wish for me in the
IOCConfig.cs
to createScreenPointFactory
as followsnew ScreenPointFactory(new ScreenTransformation())
to where instead of being a No-Op type operation it does the work of conversion, and thatArbitraryPointFactory
had the No-Op. I would then be able to pass in theX,Y
or perhaps theIPoint
interface to theScreenPointFactory
or the correctPointFactory
method to then do some conversion fromArbitraryPoint
perhaps to what could be aScreenPoint
. So in this instance, I would be modifying the data of anArbitraryPoint
that may be a point on a video game model, and then use myScreenPointFactory
to create aScreenPoint
from anArbitraryPoint
which does all the coordinate conversions. Then instead of modifying theScreenPoint
calcs directly, I would still be modifying the video game model directly or the originalArbitraryPoint
, to then throw away my generatedScreenPoint
and generate a newScreenPoint
by calling theScreenPointFactory
to update myScreenPoint
location from the values of the originalArbitraryPoint
. Dealing with these complexities will definitely warrent the use of the techniques I am presenting here with Factories. Also simplifying your object models to be used in a manner that makes sense to the application that is using it.Anyways tag me back if any of what I have said is questionable or if you would like me to change the sample code around in any way to match my last paragraph. The only other way I might stick to something similar to what you have in your example is to try and implement the Adapter pattern. Then I have the ability to go from one interface to another for instance, so from
ScreenPoint
toArbitraryPoint
and vice versa without forcing them to derive from the same interface or base class. Yet providing some calculation and conversion back and forth. My implementing code would always look to the Adapter class to pull back the data I wish to operate on and then convert back and forth. Either way I guess the main design principles I have tried to express are (Found in the book Head First Design Patterns):Has-A
is better thanIs-A
)Funny thing is the book goes forward to mention for
Dependency Inversion Principle
, that if you follow all three of those design guidelines with no exceptions that you would never be able to write a single program. So I will say to you that try to aim towards these principles, but only where they make the best sense. For any of the design patterns or principles there are both pros and cons. However, at least with the design patterns they are more like templates to do what you want from already solved problems. So plug them in and adapt them where they make sense to your design. Others will be familiar with your design and can jump right in with your naming strategy and add to your code.