Best way to display domain object summary information efficiently and in an OO way from a large inheritance tree?

Question 1

I've provided only simplified code as it's more of an abstract design question.

So I have many, many nested business/domain event objects, e.g.

 public class Event
 {
 //bunch of properties and standard accessors
 }
 public class ExplosionEvent extends Event
 {
 //properties and standard accessors 
 }

And many more of these at different levels. If I need information about any given object chosen, I display it in HTML like so

 private String generateHTML(Event event)
 { 
 StringBuilder sb = new StringBuilder();
 sb.append("<HTML><table>");//simplified
 sb.append("<TH>Time</TH>"); 
 sb.append("<TD>" + event.getEventTime() + "</TD>");
 if (event instanceof ExplosionEvent)
 {
 //append HTML and ExplosionEvent specific data
 }
 // ...many, many more calls like the one above
 sb.append("</table></HTML>");
 }

As I have many event types, this means loads of duplicated HTML table tags and uses of instanceof so generateHTML is hundreds of lines long, split into methods of course but still, it's a lot of code that makes this hard to understand, navigate and therefore maintain.

This is ugly and I need a better design for this. I had the idea of creating a method on Event which is overridable by all sub methods

//Using LinkedHashMap to preserve order as Properties wont do that
public LinkedHashMap<String, String> getAttributes()
{
 LinkedHashMap<String, String> list = new LinkedHashMap<String, String>();
 list.put("Time", eventTime);
}

sub-classes then override this, call their parent and add date specific to them to the list meaning that no matter how many Event classes there is, the existing generateHTML method (external to Event objects) will then simply be one small loop

private String generateHTML(Event event)
{
 StringBuilder sb = new StringBuilder();
 b.append("<HTML><table>");
 for (Entry<String,String> entry : event.getAttributes().entrySet())
 {
 sb.append("<TH>" + entry.getKey() + "</TH>"); 
 sb.append("<TD>" + entry.getValue() + "</TD>");
 }
 sb.append("</table></HTML>");
}

Is this putting too much logic in business/domain objects?

Is there a better way?

Question 2

Nobody on Programmers have some ideas?

Question 3

This is exactly the issue that OCP (Open-Closed Principle) addresses. This is well documented and I suggest looking this up rather than having someone here regurgitate this well-known good practice advice.

Question 4

As an aside, your code is not "nested". Inheritance graphs are not the same as nesting.

Question 5

This looks suspicious to me ...

if (event instanceof ExplosionEvent)
{
 //append HTML and ExplosionEvent specific data
}

... usually when I see that I think that Event ought to be abstract with an overridable method. That method be LinkedHashMap<String, String> getAttributes() as you suggested.

Another possibility is to store the LinkedHashMap as a member of Event (pseudo-code ahead):

class Event
{
 LinkedHashMap<String, String> map;
 protected void SetAttribute(String name, String value)
 {
 // stores in map
 }
 protected String getAttribute(String name)
 {
 // gets from map
 }
 LinkedHashMap<String, String> getAttributes()
 {
 // returns entire map
 }
}
class ExplosionEvent inherits Event
{
 String getLocation() { return getAttribute("location"); }
 void setLocation(String location) { setAttribute("location", location); }
}

You could consider using the java.utils.Properties for your implementation.

Another solution might be to use reflection.

Question 6

I hate instanceof myself, that and the duplicated table HTML. It's been suggested by colleagues that my solution is wrong because it places processing into objects which is wrong.

Question 7

If someone adds a new property to the ExplosionEvent subclass then they'll need to edit your generateHTML method too, which IMO is <strike>no better</strike> worse. To be 'object oriented' the base Event class should either contain (see java.utils.Properties) or abstract the list of properties; or use reflection; or (similar to refelction) use auto-generated ORM code somehow (define properties as XML and auto-gererate the Java code for the event subclasses and the generateHTML method).

Question 8

This is a bad solution because it mixes two completely separate concerns - your objects' primary functions, and outputting its properties as HTML. On the other hand, I imagine you're trying to get access to the object's private state, which violates abstraction.

If you're only trying to print out this HTML for debugging purposes, the best solution would be to use reflection to inspect its fields/properties. If you only need to print out some fields, you can use annotations to mark which fields are meant to be inspected with reflection.

If providing access to its fields is part of the design then your classes should provide a your getAttributes as part of an interface, but delegate generateHTML to someone else. That way you don't have to change your classes just because you want to change the HTML formatting. You can decouple the code further by returning Map or Iterator from getAttributes instead of LinkedHashMap.

Question 9

This is part of the design. In my solution mentioned I have a getAttributes function per Event which returns attributes as key and value pair. In the part currently generating HTML, it will still do so, just doing it as a loop using the key and value pair from each event.

Question 10

A pretty obvious, object-oriented and simple way would be to add the generateHtml() method into the Event objects.

I.e. have the events generate their own view. I would perhaps not call it generateHtml(), but something like displaySummary(). That fits better with what it does, and not how it does it, or with which technology.

If you have commonalities between events, you could try to create a common display widget for it. For example DataPanel to display label-value pairs, etc.

Question 11

The simple solution of implementing generateHTML() and overriding it in subclasses is indeed a common object oriented approach, but as noticed, it puts unrelated logic or functionality into business objects.

This can be avoided by implementing a double dispatch which is essentially a simplified version of the visitor pattern:

Implement an abstract class (or interface) EventVisitor with methods visitEvent(), visitExplosionEvent() etc. and a class EventHTMLGenerator which subclasses the superclass or implements the interface. Of course, the visitor class must know about the visited objects, and depending on your language it must be a friend class or the Event attributes must be otherwise accessible to it.
In your Event class, implement an accept() method that takes a Visitor argument and calls its visitEvent() method. Override accept() in each subclass to call the appropriate visitX() method

The visitor pattern is often used to traverse recursive object structures such as syntax trees, but this doesn't apply here, so your visitX() methods would not need to call accept() recursively.

Of course, this leads to somewhat more code, but it keeps everything related to HTML generation for Events in one place.

ChrisW ChrisW 3,4272 gold badges22 silver badges29 bronze badges · Answer 1 · 2014-01-31 13:55:21Z

This looks suspicious to me ...

if (event instanceof ExplosionEvent)
{
 //append HTML and ExplosionEvent specific data
}

... usually when I see that I think that Event ought to be abstract with an overridable method. That method be LinkedHashMap<String, String> getAttributes() as you suggested.

Another possibility is to store the LinkedHashMap as a member of Event (pseudo-code ahead):

class Event
{
 LinkedHashMap<String, String> map;
 protected void SetAttribute(String name, String value)
 {
 // stores in map
 }
 protected String getAttribute(String name)
 {
 // gets from map
 }
 LinkedHashMap<String, String> getAttributes()
 {
 // returns entire map
 }
}
class ExplosionEvent inherits Event
{
 String getLocation() { return getAttribute("location"); }
 void setLocation(String location) { setAttribute("location", location); }
}

You could consider using the java.utils.Properties for your implementation.

Another solution might be to use reflection.

I hate instanceof myself, that and the duplicated table HTML. It's been suggested by colleagues that my solution is wrong because it places processing into objects which is wrong.
If someone adds a new property to the ExplosionEvent subclass then they'll need to edit your generateHTML method too, which IMO is <strike>no better</strike> worse. To be 'object oriented' the base Event class should either contain (see java.utils.Properties) or abstract the list of properties; or use reflection; or (similar to refelction) use auto-generated ORM code somehow (define properties as XML and auto-gererate the Java code for the event subclasses and the generateHTML method).

Doval Doval 15.5k3 gold badges46 silver badges59 bronze badges · Answer 2 · 2014-02-10 19:01:27Z

This is a bad solution because it mixes two completely separate concerns - your objects' primary functions, and outputting its properties as HTML. On the other hand, I imagine you're trying to get access to the object's private state, which violates abstraction.

If you're only trying to print out this HTML for debugging purposes, the best solution would be to use reflection to inspect its fields/properties. If you only need to print out some fields, you can use annotations to mark which fields are meant to be inspected with reflection.

If providing access to its fields is part of the design then your classes should provide a your getAttributes as part of an interface, but delegate generateHTML to someone else. That way you don't have to change your classes just because you want to change the HTML formatting. You can decouple the code further by returning Map or Iterator from getAttributes instead of LinkedHashMap.

This is part of the design. In my solution mentioned I have a getAttributes function per Event which returns attributes as key and value pair. In the part currently generating HTML, it will still do so, just doing it as a loop using the key and value pair from each event.

score 1 · Answer 3 · 2021-08-08 12:08:11Z

A pretty obvious, object-oriented and simple way would be to add the generateHtml() method into the Event objects.

I.e. have the events generate their own view. I would perhaps not call it generateHtml(), but something like displaySummary(). That fits better with what it does, and not how it does it, or with which technology.

If you have commonalities between events, you could try to create a common display widget for it. For example DataPanel to display label-value pairs, etc.

score 0 · Answer 4 · 2021-08-08 14:46:40Z

The simple solution of implementing generateHTML() and overriding it in subclasses is indeed a common object oriented approach, but as noticed, it puts unrelated logic or functionality into business objects.

This can be avoided by implementing a double dispatch which is essentially a simplified version of the visitor pattern:

Implement an abstract class (or interface) EventVisitor with methods visitEvent(), visitExplosionEvent() etc. and a class EventHTMLGenerator which subclasses the superclass or implements the interface. Of course, the visitor class must know about the visited objects, and depending on your language it must be a friend class or the Event attributes must be otherwise accessible to it.
In your Event class, implement an accept() method that takes a Visitor argument and calls its visitEvent() method. Override accept() in each subclass to call the appropriate visitX() method

The visitor pattern is often used to traverse recursive object structures such as syntax trees, but this doesn't apply here, so your visitX() methods would not need to call accept() recursively.

Of course, this leads to somewhat more code, but it keeps everything related to HTML generation for Events in one place.

Stack Exchange Network

Best way to display domain object summary information efficiently and in an OO way from a large inheritance tree?

4 Answers 4

Your Answer

Sign up or log in

Post as a guest

Post as a guest

Hot Network Questions

Best way to display domain object summary information efficiently and in an OO way from a large inheritance tree?

4 Answers 4

Your Answer

Sign up or log in

Post as a guest

Post as a guest

Related

Hot Network Questions