35

I have two parallel inheritance chains:

Vehicle <- Car
        <- Truck <- etc.

VehicleXMLFormatter <- CarXMLFormatter
                    <- TruckXMLFormatter <- etc.

My experience has been that parallel inheritance hierarchies can become a maintenance headache as they grow.

i.e. NOT adding toXML(), toSoap(), toYAML() methods to my principal classes.

How do I avoid a parallel inheritance hierarchy without breaking the concept of separation of concerns?

Ravindra babu
  • 37,698
  • 11
  • 250
  • 211
parkr
  • 3,188
  • 5
  • 35
  • 39

6 Answers6

13

I am thinking of using the Visitor pattern.

public class Car : Vehicle
{
   public void Accept( IVehicleFormatter v )
   {
       v.Visit (this);
   }
}

public class Truck : Vehicle
{
   public void Accept( IVehicleFormatter v )
   {
       v.Visit (this);
   }
}

public interface IVehicleFormatter
{
   public void Visit( Car c );
   public void Visit( Truck t );
}

public class VehicleXmlFormatter : IVehicleFormatter
{
}

public class VehicleSoapFormatter : IVehicleFormatter
{
}

With this, you avoid an extra inheritance tree, and keep the formatting logic separated from your Vehicle-classes. Offcourse, when you create a new vehicle, you'll have to add another method to the Formatter interface (and implement this new method in all the implementations of the formatter interface).
But, I think that this is better then creating a new Vehicle class, and for every IVehicleFormatter you have, create a new class that can handle this new kind of vehicle.

Frederik Gheysels
  • 56,135
  • 11
  • 101
  • 154
  • 1
    It might be better to rename IVehicleFormatterVisitor to just IVehicleVisitor, as it is a more generic mechanism than just for formatting. – Richard Mar 30 '09 at 08:59
  • 1
    Isnt this just robbing peter to pay paul? so now you need a method in IVehicleFormatter and all implementations for each subclass of Vehicle... so what has this achieved? – Jordie Mar 30 '09 at 09:45
  • @Jordie this may be solved using AbstractVehicleVisitor which will implement the interface IVehicleVisitor. All the methods could be implemented to throw unsupported opertion exceptions. Concrete visitors would extend the abstract class and override the needed method used in their case – Boris Pavlović Mar 30 '09 at 09:52
  • 5
    But that only solves the compiler error. It doesn't actually fix the logic, and could be considered worse since now the compiler won't tell you that you forgot to implement something. – Robin Mar 30 '09 at 13:51
  • I want to know that why the `Vehicle` need to `Accept` an `IVehicleFormatter` and call it's `Visit` method instead of creating an `IVehicleFormatter` in outside and call it `Visit(vehicle)` method directly? – hao li Dec 19 '19 at 13:12
9

Another approach is to adopt a push model rather than a pull model. Typically you need different formatters because you're breaking encapsulation, and have something like:

class TruckXMLFormatter implements VehicleXMLFormatter {
   public void format (XMLStream xml, Vehicle vehicle) {
      Truck truck = (Truck)vehicle;

      xml.beginElement("truck", NS).
          attribute("name", truck.getName()).
          attribute("cost", truck.getCost()).
          endElement();
...

where you're pulling data from the specific type into the formatter.

Instead, create a format-agnostic data sink and invert the flow so the specific type pushes data to the sink

class Truck  implements Vehicle  {
   public DataSink inspect ( DataSink out ) {
      if ( out.begin("truck", this) ) {
          // begin returns boolean to let the sink ignore this object
          // allowing for cyclic graphs.
          out.property("name", name).
              property("cost", cost).
              end(this);
      }

      return out;
   }
...

That means you've still got the data encapsulated, and you're just feeding tagged data to the sink. An XML sink might then ignore certain parts of the data, maybe reorder some of it, and write the XML. It could even delegate to different sink strategy internally. But the sink doesn't necessarily need to care about the type of the vehicle, only how to represent the data in some format. Using interned global IDs rather than inline strings helps keep the computation cost down (only matters if you're writing ASN.1 or other tight formats).

Pete Kirkham
  • 48,893
  • 5
  • 92
  • 171
2

You can use Bridge_pattern

Bridge pattern decouple an abstraction from its implementation so that the two can vary independently.

enter image description here

Two orthogonal class hierarchies (The Abstraction hierarchy and Implementation hierarchy) are linked using composition (and not inheritance).This composition helps both hierarchies to vary independently.

Implementation never refers Abstraction. Abstraction contains Implementation interface as a member (through composition).

Coming back to your example:

Vehicle is Abstraction

Car and Truck are RefinedAbstraction

Formatter is Implementor

XMLFormatter, POJOFormatter are ConcreteImplementor

Pseudo code:

 Formatter formatter  = new XMLFormatter();
 Vehicle vehicle = new Car(formatter);
 vehicle.applyFormat();

 formatter  = new XMLFormatter();
 vehicle = new Truck(formatter);
 vehicle.applyFormat();

 formatter  = new POJOFormatter();
 vehicle = new Truck(formatter);
 vehicle.applyFormat();

related post:

When do you use the Bridge Pattern? How is it different from Adapter pattern?

Ravindra babu
  • 37,698
  • 11
  • 250
  • 211
2

You could try to avoid inheritance for your formatters. Simply make a VehicleXmlFormatter that can deal with Cars, Trucks, ... Reuse should be easy to achieve by chopping up the responsibilities between methods and by figuring out a good dispatch-strategy. Avoid overloading magic; be as specific as possible in naming methods in your formatter (e.g. formatTruck(Truck ...) instead of format(Truck ...)).

Only use Visitor if you need the double dispatch: when you have objects of type Vehicle and you want to format them into XML without knowing the actual concrete type. Visitor itself doesn't solve the base problem of achieving reuse in your formatter, and may introduce extra complexity you may not need. The rules above for reuse by methods (chopping up and dispatch) would apply to your Visitor implementation as well.

eljenso
  • 16,789
  • 6
  • 57
  • 63
1

Why not make IXMLFormatter an interface with toXML(), toSoap(), to YAML() methods and make the Vehicle, Car and Truck all implement that? What is wrong with that approach?

daanish.rumani
  • 317
  • 1
  • 5
  • 5
    Sometimes nothing. Other times you don't want your vehicle class to have to know about XML/SOAP/YAML - you want it to concentrate on modelling a vehicle, and to keep the markup representation separate. – Dan Vinton Mar 30 '09 at 08:43
  • 5
    It breaks the notion that a class should have a single responsibility. – parkr Mar 30 '09 at 09:19
  • 1
    It's all about *cohesion*. There are often multiple aspects or traits of an entity that pull its design in different directions, e.g. "One Fact In One Place" vs "Single Responsibility Principle". Your job as a designer is to decide which aspect "wins" by maximising cohesion. Sometimes, toXML(), toSOAP(), etc. will make sense, but in bigger, multi-tiered systems, it's better to keep presentation logic separate from the model, i.e. the aspect of presentation in a specific format (XML, SOAP, etc.) is more cohesive *across entities* as a tier, rather than tied to the specifics of each entity. – Cornel Masson Dec 22 '15 at 07:50
0

I want to add generics to Frederiks answer.

public class Car extends Vehicle
{
   public void Accept( VehicleFormatter v )
   {
       v.Visit (this);
   }
}

public class Truck extends Vehicle
{
   public void Accept( VehicleFormatter v )
   {
       v.Visit (this);
   }
}

public interface VehicleFormatter<T extends Vehicle>
{
   public void Visit( T v );
}

public class CarXmlFormatter implements VehicleFormatter<Car>
{
    //TODO: implementation
}

public class TruckXmlFormatter implements VehicleFormatter<Truck>
{
    //TODO: implementation
}
recke96
  • 170
  • 3
  • 8