Is it acceptable to return a canonical representat

2019-04-11 08:41发布

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.

3条回答
Deceive 欺骗
2楼-- · 2019-04-11 09:04

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:

public interface IScreenPointProvider
{
    ScreenPoint ToPoint();
}

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 a is-a relationship. That typically indicates that the base class really is a utility class or that the defined hierarchy isn't well designed.

查看更多
Deceive 欺骗
3楼-- · 2019-04-11 09:14

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 and ScreenPoint as different classes is correct. But in fact they "work together": All AbstractPoints should be able to convert to ScreenPoint (that's maybe the most important functionality in AbstractPoint's contract?). Since one cannot exist without the other, there is nothing wrong about AbstractPoint knowing about ScreenPoint.

Update

In a different design: Create an interface called CanonicalPoint. AbstractPoint has a method called ToCanonicalPoint, which returns CanonicalPoint. All derived classes of AbstractPoint have to implement this and return CanonicalPoint. ScreenPoint is a derived class of AbstractPoint which implements the CanonicalPoint interface. You could even have more than one derived class that implements CanonicalPoint. Note: If AbstractPoint and CanonicalPoint have methods in common, both can implement another interface called, say, Pointable, that declares all of these methods.

查看更多
爱情/是我丢掉的垃圾
4楼-- · 2019-04-11 09:17

I feel that the Single responsibility principle in SOLID has been overlooked. The AbstractPoint and its concrete implementation ScreenPoint, are essentially storing data, e.g. X, Y, unique to the class group. As well the base class AbstractPoint 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 the AbstractPoint and/or ScreenPoint classes; I feel like a separate ScreenPointFactory class implementing Factory Method pattern to instantiate the ScreenPoint is much needed here.

If you are needing to create thousands of ScreenPoint classes, having the extra virtual method calls such as ToScreenPoint() 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 an IPoint interface and simply derive AbstractPoint off of the interface IPoint. IPoint will simply hold X, Y for now. The remaining point types gain the IPoint interface through inheritance of AbstractPoint. 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 remove ToScreenPoint() from the Point classes completely and instead, I would create an ArbitraryTransformation object configured upon startup. I would use Dependency Injection to place the ArbitraryTransformation 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 the AbstractFactory will use the appropriately configured ArbitraryTransformation 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, the Object Pool pattern as used in the ScreenPointFactory is a mere functional place holder.

I have updated the code in the Point Example to reflect naming things Flyweight pattern to Object 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 of Flyweight pattern to the code. Regardless of my naming mishap, both Object Pool pattern and Flyweight 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 of Object 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 gave ArbitraryPointFactory 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 the new 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 different ScreenPoints. How would you incorporate this into your design? – Miroslav Policki

Answer: 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 suggesting ScreenPoint 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 of ScreenPoint 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 away ScreenPoint 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 renders X,Y data, rather than just creating a new set of ScreenPoint objects for each additional render. Now that can seem wasteful, and Flyweight pattern could help to overcome having to create so many new objects at runtime as there would be only one ScreenPoint object, and the implemenation of each X,Y would be encapsulated to an internal Array or Dictionary. 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 of ScreenPoint objects very quickly when thinking of my design. The design allows ScreenPoint 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 the AbitraryTransformation object with your ArbitraryPoint object or a NoOpTransformation with your ScreenPoint to be utilized in object properties or mutator methods to perform the calculation from within the ArbitraryPoint or ScreenPoint objects respectively. This is where Has-A is better than Is-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 a MochTransformation object in unit tests with a MochScreenPointFactory to be able to actually test a ScreenPoint 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 the ToScreenPoint() method on your ScreenPoint 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 the ScreenPoint 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 for ScreenPoint in two places, or each ToScreenPoint method. With the appropriate injected factory into each point, the factory method could be called from the ToScreenPoint() method of each type of point. There is nothing preventing this either, and there is some other point the code will be instantiating the first ScreenPoint or AbstractPoint which could be delegated to a factory instead of setup in each location a ScreenPoint needs to be created. Also, I do not like how the AbstractPoint class is returning a single concrete instance of ScreenPoint rather than an interface. You have simply embedded the notion of an Abstract 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 that ScreenPoint and ArbitraryPoint derived from the same interface and yet ArbitraryPoint 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 the ArbitraryTransformation 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 create ScreenPointFactory as follows new ScreenPointFactory(new ScreenTransformation()) to where instead of being a No-Op type operation it does the work of conversion, and that ArbitraryPointFactory had the No-Op. I would then be able to pass in the X,Y or perhaps the IPoint interface to the ScreenPointFactory or the correct PointFactory method to then do some conversion from ArbitraryPoint perhaps to what could be a ScreenPoint. So in this instance, I would be modifying the data of an ArbitraryPoint that may be a point on a video game model, and then use my ScreenPointFactory to create a ScreenPoint from an ArbitraryPoint which does all the coordinate conversions. Then instead of modifying the ScreenPoint calcs directly, I would still be modifying the video game model directly or the original ArbitraryPoint, to then throw away my generated ScreenPoint and generate a new ScreenPoint by calling the ScreenPointFactory to update my ScreenPoint location from the values of the original ArbitraryPoint. 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 to ArbitraryPoint 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):

  • Program to an interface, not an implemenation.
  • Favor composition over inheritance. (Has-A is better than Is-A)
  • Dependency Inversion Principle (Depend upon abstractions. Do not depend upon concrete classes.)
    • No variable should hold a reference to a concrete class. (If you use new, you'll be holding a reference to a concrete class. Use a factory to get around that!)
    • No class should derive from a concrete class. (If you derive from a concrete class, you're depending on a concrete class. Derive from an abstraction, like an interface or an abstract class.)
    • No method should override an implemented method of any of its base classes. (If you override an implemented method, then your base class wasn't really an abstraction to start with.)

      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.
查看更多
登录 后发表回答