Reputation: 2922
I am designing an application on paper at the moment and I'm in a bit of a pickle design wise.
The goal of the application is that I have car factories that can give me blueprints of their car models. A blueprint is not a car but it describes the eventual car that can be built form it, e.g., model name, options on the vehicle like airconditioning etc.
For this I have created an interface ICarBlueprint
. The methods on this interface could include getModelName()
or getOptionList()
for example. These methods have to work on each instance of car blueprint, be it an Opel or a Volkswagen car blueprint.
A factory, e.g., Opel or Volkswagen, can give a list of their car blueprints. For factories I have thus created an interface ICarFactory
. This interface contains the method getAllBlueprints()
which returns a List
of ICarBluePrint
objects. For the Opel factory this would then be OpelBlueprint
instances, which implement the interface ICarBlueprint
.
In my main program I want to get all the possible car blue prints that a user can pick from. An example could be:
for(ICarFactory f : factories)
allblueprints.addAll(f.getAllBlueprints());
I then want to iterate over that list and if one of those blueprints is the car I want, I want to build it. However, a car blue print can only be built by the factory that has generated this blueprint. So a blueprint of an Opel car can only be built by an instance of OpelFactory
, which would thus implement the interface ICarFactory
.
Building a car would result in an actual car object. For this I have created the interface ICar
. Again, for each factory I would have classes that implement this interface.
The design issue Im having starts here.
To build a blueprint I can add a method named build()
to the ICarFactory
interface. The main program can then simply pick a car instance, e.g., goodcarBlueprint
and ask a car factory to build it.
So in order to make sure that a blueprint is built by the correct factory I was thinking about adding a method getFactory()
to the interface ICarBlueprint
and build()
to the ICarFactory
interface. This would then allow me to build a blueprint in its respective factory like so:
ICarBlueprint goodcarBlueprint = // Assigned somewhere else
ICar x = goodcarBlueprint.getFactory().build(goodcarBluePrint);
However, my gut feeling is telling me that this really bad design. Any help about the proper way to design this is appreciated.
Upvotes: 2
Views: 106
Reputation: 1738
The thing I would avoid is ICarBlueprint
and ICarFactory
knowing about one other. That tight coupling would complicate future changes where you might have multiple factories that could build the same car blueprint. Loose coupling opens up possibilities.
public interface ICarBuilder {
ICarBlueprint getBlueprint();
ICar build();
}
public class CarBuilder implements ICarBuilder {
private final ICarBlueprint blueprint;
private final Collection<ICarFactory> factories = new ArrayList<ICarFactory>();
public CarBuilder(ICarBlueprint blueprint) {
this.blueprint = blueprint;
}
public ICarBlueprint getBlueprint() {
return blueprint;
}
public CarBuilder addFactory(ICarFactory factory) {
factories.add(factory);
return this;
}
public ICar build() {
// Choose least busy or least cost factory, etc. from factories.
ICarFactory factory = ...;
return factory.build(blueprint);
}
}
Now you can create a collection of ICarBuilder
and show the user the blueprints, then let the CarBuilder
decide which factory to use to build the selected car. You could even add logic to show the user factory options based on delivery time and cost and let them choose.
Upvotes: 2
Reputation: 24187
Ok, here's my tuppence worth, it could be you are violating the Law of Demeter.
The complexity of this design is due to two requirements. 1. The blueprint must know what factory it belongs to. You have a public method getFactory in your design. Does the client of the blueprint need to know the whole factory? We could just add a build method to the blueprint that calls the factory's build internally, this improves information hiding and encapsulation.
ICar x = goodcarBlueprint.getFactory().build(goodcarBluePrint);
gets reduced to
ICar x = goodcarBlueprint.build();
because we have this extra method in the blueprint class
public void Build()
{
this.factory.Build(this.BlueprintId);
}
It is always good to provide just the information that is required and no more; you don't give a wallet to a cashier, you give them just the money they require. However, if you do want the whole Factory for other parts of the application then your original design was fine but exposing it could create an additional dependency and make it difficult to change factory implementations at a later date as the clients will begin to depend on it.
Another constraint that is mentioned is this - 2. each blueprint must only be used by one type of factory. I'm not sure how much you want to constrain this, but if you do...you could return a list of IBlueprints from each factory but only accept the specific type of blueprint into a factory.
class OpelFactory : IFactory
{
public IBluePrints GetBluePrints()
{
return this.bluePrints;
}
public void AddBluePrint(IOpelBluePrint bluePrint) //1. we can only add opel blueprints into the opel factory
{
this.bluePrints.Add(bluePrint);
}
Upvotes: 1
Reputation: 12022
This is very subjective and I understand that feeling of bad design, but I would not call what you came up with "really bad".
To me it sounds like a classic Builder design pattern. Where the Blueprint is the Builder in this case. Most Builders have a .build() method. By calling .build() on the Blueprint you would avoid having to retrieve the Blueprint from the factory to just call the factory again with the Blueprint that it gave you. The Blueprint would still have to know about its own Factory so that it can have the Factory build the car.
Upvotes: 2
Reputation: 3200
I think your design is fine. The line :
ICar x = goodcarBlueprint.getFactory().build(goodcarBluePrint);
Is a bit verbose and you probably don't like the fact that goodcarBluePrintFactory appears twice there. There is a simple solution for that, keep your desing and add this static helper function to ICarFactory :
public static ICar build( ICarBlueprint blueprint) {
return blueprint.getFactory().build(blueprint);
}
And when you want to build a car from a blueprint you will just :
ICar x = ICarFactory.build(goodcarBluePrint);
Or maybe you could instead add a build method to ICarBlueprint :
public Icar build() {
getFactory().build(this);
}
Which you would use as :
ICar x = goodcarBlueprint.build();
I think I like the last better.
Upvotes: 1