HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Design of class hierarchy for a object formatter API forces derivatives to know too much

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
muchknowhierarchyformatterdesignderivativestooapiforcesfor

Problem

While learning object oriented design I'm judging my own design critically. This framework should be able to print objects in either XML, or JSON, I've stubbed in a basic implementation to avoid getting into details of XML and JSON parser APIs for now.

So I made the Formatter be the base class. But with my current design, all derivatives of this base class would need to know that they have to call: getFormattedValue() to get output. Also I don't feel comfortable with all of those if else statements in the Formatter constructor. The clients would need to know to pass in either an "XML" or "JSON" in all derivatives of this class. How can I improve this design to conform to all object oriented design principles?

public class Formatter {

    private String output;

    public Formatter(Object object, String formatType){
        if(formatType.equals("xml")){
            output = getXMLFormat(object);
        } else if(formatType.equals("json")) {
            output = getJSONFormat(object);
        }
    }

    private String getXMLFormat(Object object){

        return ""+object.toString()+""; // simplified
    }

    private String getJSONFormat(Object object){
        return "{"+object.toString()+"}"; // simplified
    }

   protected  String getFormattedValue(){
        return output;
    }
}


The derivative class:

public class ItemFormatter extends Formatter {

    public ItemFormatter(Employee item, String formatOutput) {
        super(item, formatOutput);
    }

    public void printItem(){
        System.out.println(getFormattedValue());
    }
}

Solution

This one is screaming for an enum. What you want is what you get:

public enum FormatType {
    XML, JSON, //maybe further support later
}


And your constructor then becomes:

public Formatter (Object object, FormatType formatType) {
    switch (formatType) {
        case XML:
            output = getXMLFormat(object);
            break;
        case JSON:
            output = getJSONFormat(object);
            break;
    }
}


In addition to that a small nitpick on naming:

Your get...Format() methods break the camelCase you kept until then. I suggest you ignore that these formats are acronyms and instead use following method names:

getXmlFormat (Object object);
getJsonFormat (Object object);


But I'd say that's personal preference. All else you didn't give us much to review :(

Code Snippets

public enum FormatType {
    XML, JSON, //maybe further support later
}
public Formatter (Object object, FormatType formatType) {
    switch (formatType) {
        case XML:
            output = getXMLFormat(object);
            break;
        case JSON:
            output = getJSONFormat(object);
            break;
    }
}
getXmlFormat (Object object);
getJsonFormat (Object object);

Context

StackExchange Code Review Q#60459, answer score: 5

Revisions (0)

No revisions yet.