patternjavaMinor
Design of class hierarchy for a object formatter API forces derivatives to know too much
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
The derivative class:
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:
And your constructor then becomes:
In addition to that a small nitpick on naming:
Your
But I'd say that's personal preference. All else you didn't give us much to review :(
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.