views:

130

answers:

3

I have 5 different tables in a database. I have written an abstract "Converter.java" class that take out data from the database and convert it into a "tree.xml" file.

Tree.xml

<?xml version="1.0" standalone="no"?>
<tree>
   <declarations>
       <attributeDec1 name="name" type="String"/>
   </declarations>

   <branch>
       <attribute name="name" value="process 1"/>

       <leaf>
           <attribute name="name" value="process 2"/>
       </leaf>

       <leaf>
           <attribute name="name" value="process 3"/>
       </leaf>
   </branch>
</tree>

So as you can guess, the structure of the above tree.xml will remain same for all the 5 tables. The only difference is in the value of attribute "value".

To get the value of that "value" attribute, the converter first have to query the database for those values.

So, there should be 5 different queries for 5 different tables. So instead of coding 5 different converters, I have made a singele "Converter.java" that has functions like

  • openTree()
  • closeTree()
  • openBranch()
  • closeBranc()
  • openLeaf()
  • closeLeaf()
  • addAttribute()

Out of the above methods, I have implemented all the methods except addAttribute() method as it will have implementation depending on the table.

After that, I have coded 5 different converters, each for one table and all of them extends "Converter.java"

I have made a field named as "query" in the Converter.java class. As this field is inherited by all the 5 Converters, I have initialized this field in the constructors of those 5 converters.

Now my question is as follows:

Q. I am not confident that whether I have used inheritance correctly or there should be some changes in the above approach?

Edit:

Q2. I have one more method in Converter.java class that uses "query" field. As the field is not initialized in the Converter.java class, so whether I copy that method in all the 5 child converters or it is okay to put that method in the parent class (i.e. Converter.java)

+1  A: 

Hopefully I understand your question correctly. If I do, then it sounds like you are directly initializing the field like "mQuery = foo;" in each constructor? If that is the case, then probably the best form would be to instead initialize it only in the superclass constructor, and then invoke that superclass constructor in each subclass constructor like "super(foo);".

spurserh
+3  A: 

I think the Strategy pattern would be more appropriate here. You may create a non-abstract Converter class with the common behavior, and supply it with the ValueSource, which can perform a table-specific value retrieval. Then you create a QueryValueSource (a subclass of ValueSource) to retrieve a value based on the specified query.

To get a concrete strategies, you can either pass different queries into a constructor of QueryValueSource, or subclass it to create table-specific sources.

axtavt
This is definitely the way to go. Too often people try to use inheritance as a its own design-pattern and while it can work, choosing a design-pattern that matches what you're trying to do can provide significant value in design, development, maintenance, and when you are trying to add enhancement functionality.
BestPractices
A: 

Regarding question Q2: what you can do is make the Converter class abstract and provide implementation for the method that uses the "query" attribute in it. Have the constructor of the Converter subclasses to initialize the query attribute.

Regarding Q1, I am not able to fully understand the question with the context you have provided. May be some code sample would help. But with this this limited understanding what I can suggest is did you consider whether using composition instead of inheritance would have helped ? Also it seems to me that the Strategy pattern can be of use here.

sateesh