0

I'm working on an existing code base and am trying to reduce a lot of duplication and gain some polymorphic behavior. Right now I have a type of domain object with an abstract base class and two (at the moment) subclasses. In our domain we have a need sometimes to create pdf documents of these objects and send them as mail attachments, write them over HTTP, etc. The appearance of the pdf will of course differ a bit between the subclasses, but I don't feel like the domain objects should know about pdf files or how they should render and so on.

I have extracted an interface for building pdf documents from these objects. This interface is implemented by one class per subclass of the domain object, and I also have a factory method that decides what implementation should be used. Currently it makes this decision by checking the runtime type of the object, and if nothing matches, it just returns a "null builder" that creates an empty pdf.

I think I am on to something in the sense that I have achieved some SoC and deleted a lot of duplication, but I am not sure if the design is sound. I kind of get a knee-jerk reaction from checking runtime types for one thing, but something tells me that the fact that it happens in a factory and that I have a null implementation to return makes it more OK.

If I were to write all of this from scratch today I would have gone down a different route entirely, but like everyone else I have limited time and I wanna go with the "2 minutes to better code" approach, meaning do improvements in small steps every now and then and the code will improve over time. I'm not very familiar with the pattern, but I think maybe a visitor pattern could be of use here?

4
  • Is there a reason you can't configure the domain objects with the appropriate pdf builder (or a factory that builds one directly) when they are initially created (at which point you presumably already know their type), this decreasing the number of places that need to change if the hierarchy is modified?
    – Jules
    Commented Dec 11, 2015 at 21:58
  • well, it'd be rather bothersome. A bunch of types would have to be moved to avoid circular dependencies and frankly I don't really think the domain objects have any business knowing how to create and arrange a pdf document intended for rendering the object on a screen or for printing. it's not within their responsibility. we could change the pdf lib tomorrow. or go from pdf to something else. it's not really part of the business rules for the classes. I like something like a visitor or an injected factory more then
    – sara
    Commented Dec 12, 2015 at 22:50
  • Is returning a null builder actually desired? I would have thought that passing in an unknown type to your factory would fall under the category of undefined behaviour and, as such, should result in an exception being thrown.
    – MetaFight
    Commented Jan 26, 2016 at 22:09
  • I ended up running with a visitor-ish solution, where the entity base class exposes an abstract AsPdf(IPdfMaker) method, where the IPdfMaker interface exposes methods that accept the different subclasses, allowing for double dispatch. It works pretty good so far. I agree that a "null factory" really feels off here, since right now, getting one is never desirable.
    – sara
    Commented Jan 27, 2016 at 15:46

0

Reset to default

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Browse other questions tagged or ask your own question.