• Post Reply Bookmark Topic Watch Topic
  • New Topic

Inheritance and Abstract (code review)  RSS feed

 
Raymond Holguin
Ranch Hand
Posts: 82
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
So my situation broken down to something very simple is that I have classes that are practically identical except for a couple variables. The action classes need to be very generalized in that i could be using either one of the classes at any time because i don't want to create identical action classes just to handle each data type. So i have something something like this to start with.




Now in an attempt to generalize this i use Inheritance which makes thing a little nicer



The issue that I still have all those instanceof if/else statements make the code messy and hard to read. I need to get it even more generalized than that, so i decided to look at the Abstract class route and this is what I ended up with.



This works, but I'm not sure if this is the best way to do things. It makes my implementation code much simpler to use but having those forced abstract methods that don't really do anything just doesn't look right, but its the only solution i can think of in order to keep my code generic. Is this the correct way to do things or does anyone have a better solution? Is keeping all the instanceof methods the better way to go?

Thanks
-Ray
 
Campbell Ritchie
Marshal
Posts: 56541
172
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
What I dislike more than abstract methods is instanceof.
Your classes A and B are not properly encapsulated; their fields should all be private. I would suggest you make all your class implement a common interface (Foo), so rather than using a List<Object> and those horrible casts, create a List<Foo>.
 
Steve Luke
Bartender
Posts: 4181
22
IntelliJ IDE Java Python
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I second Campbell's suggestion of an interface, but rather than making the interface provide the value methods (what if you have different types or number of values) I would make it provide the print method. Something like:
 
Steve Luke
Bartender
Posts: 4181
22
IntelliJ IDE Java Python
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
But a good toString() method might take care of this as well, such that you could just call System.out.println(theObject); and rely on its toString() to provide the expected output.
 
Piet Souris
Master Rancher
Posts: 2044
75
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I was thinking something like:



or just using List<String> instead of a hashmap, in which case you could use


And if this looks useful, then you could even do class MotherOfAll<K, V> {...}

Greetings,
Piet
 
Raymond Holguin
Ranch Hand
Posts: 82
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks for your response, but when i use Interface instead of the Abstract class I feel like I'm still in the same boat. I still force my subclasses to implement methods that are not relevant to them. I mean if this is fine in terms of coding practices then I'll deal with it, but it just looks ugly



I second Campbell's suggestion of an interface, but rather than making the interface provide the value methods (what if you have different types or number of values) I would make it provide the print method. Something like:


The print method was more just an example, but issue is with your example is that if i have some sort of data processing class that can take either ClassA or ClassB as input and I need to access a single property of one of those objects I am forced to use "instanceof". I won't need instanceof to call this "print" method, but everything else I will.
 
Raymond Holguin
Ranch Hand
Posts: 82
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
As i mentioned in my OP, this was just an over simplified example, I am just trying to get an idea of what the best design is to implement a scenario with classes sharing multiple properties but also have their own unique properties.. The truth of the matter is that these database classes have around 20 fields each and multiple methods, some shared and some not. My implementation is much more complicated than just a print method. But i just want a simple start off point so i can go from there.
 
Steve Luke
Bartender
Posts: 4181
22
IntelliJ IDE Java Python
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Raymond Holguin wrote:The print method was more just an example, but issue is with your example is that if i have some sort of data processing class that can take either ClassA or ClassB as input and I need to access a single property of one of those objects I am forced to use "instanceof". I won't need instanceof to call this "print" method, but everything else I will.

Not if you follow it through all the way. For example, instead of printing you could do 'processing':
 
Steve Luke
Bartender
Posts: 4181
22
IntelliJ IDE Java Python
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Raymond Holguin wrote:As i mentioned in my OP, this was just an over simplified example, I am just trying to get an idea of what the best design is to implement a scenario with classes sharing multiple properties but also have their own unique properties..

The idea is to not force a complicated and contrived inheritance model, but to make something you can inject into the Objects so that the Objects themselves control how the data is used, since they have knowledge of their internals. As in my previous example, you provide an interface for the processing step that the Objects can use to sequence through the work. Then provide an interface so the caller (the print method in the original example) has a kicking off point that does not rely on the concrete type of the Object.
 
Raymond Holguin
Ranch Hand
Posts: 82
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
hey steve, thanks for your help. I understand what your saying but I believe my scenario is different and Its my fault for trying to "simplify" things because i made it appear i was trying to do something that I am really not.

So I'm going to break it down real world nitty-gritty. I have a web app with about 10 pages, the main chunk of the data shown on these pages is either 1 of 2 database classes. Which one of these classes data is being show is literally control by the checkbox. Now these classes are 80% similar in the properties they share. So currently in my servlet controller, depending on what class is chosen to be displayed i run a query and return that result set back to my JSP. The issue is that on my jsp, when im looping through the result data I have to call "instanceof" for a portion of my data output since some of the properties don't exist for one of the classes. What I am trying to do is get rid of all that and just be able to return a set of Objects, call a property getter and regardless of what class was queried at the time, it will be a valid method call and exist.

This also goes from the various forms when im saving/updating data. I dont want to have all the "instanceof" declarations when having to call my setters. In one instanceof if statement I can call 15 setters on ClassA and the else statement is the same 15 setters called on ClassB. I don't want that, especially since data is editable from multiple pages so that horrible code is everywhere. I just to have a single class i can call my setters for, pass that object to my database saving processes which can sort it out from there in that one controlled spot.

I hope this helps clear up what I am trying to do and hopefully makes more sense with what I was previously asking in my examples. My thinking is that by using the abstract classes I can achieve this single point of access and rid myself of all the instanceof operators.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!