1

I have an application which has to handle multiple types of documents.

I have a service generateReport(DocumentEntity) which has to build a report, this service will be called for every types of child of DocumentEntity.

The thing is that the report we want to build depends on the type of document.

My code bellow works but I feel like there is a better way to do it :

Lets say we have DocumentDonationEntity & DocumentBookEntity which extends DocumentEntity :

public abstract class DocumentEntity {
    protected Long id;
    protected String name;
    // getters & setters
}

public class DocumentDonationEntity extends DocumentEntity {
    private String donatorName;
    // getters & setters
}

public class DocumentBookEntity extends DocumentEntity {
    private Long price;
    // getters & setters
}

And ReportDataBook & and ReportDataDonation which extends ReportData :

public abstract class ReportData {
    protected Long id;
    protected String name;

    public void checkNotNull() throws MyException {
        if this.id == null
            throw new myException();
        if this.name == null
            throw new myException();
        // other generic stuff
    }
    // getters & setters
}

public class ReportDataBook extends ReportData {
    private String price;

    public void checkNotNull() throws MyException {
        super.checkNotNull();
        if this.price == null
            throw new myException();
        // other stuff specific to books
    }
    // getters & setters
}

public class ReportDataDonation extends ReportData {
    private String donatorName;

    public void checkNotNull() throws MyException {
        super.checkNotNull();
        if this.donatorName == null
            throw new MyException();
        // other stuff specific to donations
    }
    // getters & setters
}

Plus an utils class :

public class Utils {
    public ReportData fillReport(DocumentEntity document) throws Exception {
        if (document instanceof DocumentBookEntity) {
            ReportDataBook data = new ReportDataBook();
            completeReportData(data, (DocumentBookEntity) document);
            completeReportData(data, document);
            return data;
        } else if (document instanceof DocumentDonationEntity) {
            ReportDataDonation data = new ReportDataDonation ();
            completeReportData(data, (DocumentDonationEntity) document);
            completeReportData(data, document);
            return data;
        }
        return null;
    }

    public void completeReportData(ReportData data, DocumentEntity document) {
        // some generic stuff
    }

    public void completeReportData(ReportDataBook data, DocumentBookEntity document) {
        // some stuff specific to books
    }

     public void completeReportData(ReportDataDonation data, DocumentDonationEntity document) {
        // some stuff specific to donations
    }

}

Then I have a Service :

@Service
public class MyService implements IMyService {

    @Override
    public boolean generateReport(DocumentEntity document) throws MyException {
        ReportData data = initializeReport(document);
        // do some other stuff
    }

    private ReportData initializeReport(DocumentEntity document) throws myException {
        if (document instanceof DocumentBookEntity) {
            ReportDataBook data = (ReportDataBook) utils.fillReport(document);
            data.checkNotNull();
            return data;
        } else if (document instanceof DocumentDonationEntity) {
            ReportDataDonation data = (ReportDataBook) utils.fillReport(document);
            data.checkNotNull();
            return data;
        }
        return null;
    }
}
Ilona
  • 357
  • 3
  • 10
Adrien B.
  • 13
  • 3

1 Answers1

0

Whenever you find instanceof you can consider refactoring to polymorphism per http://refactoring.com/catalog/replaceConditionalWithPolymorphism.html

I did some UML diagrams, because I find it easier to grok a design with subclass hierarchies that way:

UML of the class hierarchies

I would consider the Factory Method pattern:

Factory Method GoF pattern in UML

I see your DocumentEntity hierarchy as the Creator in this pattern, and ReportData as the Product:

Applying Factory Method to example

The above example only shows one factory method implementation. The other one (DocumentBookEntity) would return a new ReportBookDonation().

The ReportData classes then handle all the specifics for the subtype of DocumentEntity. I don't think you really need MyService stuff anymore. MyService.initializeReport() should be moved into the ReportData hierarchy; it's now polymorhphic in the subclasses (eliminating the instanceof conditional). It's the same with the code for Utils.fillReport() - it's a service offered by the specific versions of the reports.

If the other Utils methods don't depend on the subtypes, then put them in the ReportData abstract class. Personally, I'd call that a Report class, since objects are more about behaviors (services) than data structures.

So, to generate a report, it would look like this:

DocumentEntity doc;   // it's really an instance of a subclass
...
ReportData reporter = doc.createReportService();   // factory method call (polymorphic)
...
reporter.initializeReport();  // lazy initialization

You could possibly put the initialize and fill stuff in the constructor of the subclass, which would generate the reports as soon as you call the factory method.

Community
  • 1
  • 1
Fuhrmanator
  • 11,459
  • 6
  • 62
  • 111
  • This is very similar to `Collection.iterator()` factory method, as http://stackoverflow.com/questions/31017531/please-give-an-example-of-factory-method-pattern/31018337#31018337 – Fuhrmanator Sep 18 '15 at 14:44
  • Thank you for your great answer :) I have an interrogation : If the process of generating the report needs to call external web services or need to access the DB, is it a good idea to autowire DAOs ans Services directly into my entity ? – Adrien B. Sep 19 '15 at 16:21
  • Aren't entities meant to be POJOs ? And then externalize logical processes ? – Adrien B. Sep 19 '15 at 16:29
  • @AdrienB. I don't know of a general reason why you couldn't access them directly. As for entities being POJOs, it might depend on the framework you're using. Without a framework, everything is a POJO. – Fuhrmanator Sep 19 '15 at 21:38