1

In java i am writing some DTO objects, which all inherit from AllocationDTO.
A list of these objects are then passed into a DAO object for saving to the database.

Depending on which subtype of AllocationDTO is being saved the saving logic changes (e.g. which table in the database to save to etc.)

I find myself using code as such:

for (AllocationDTO x : listOfAllocationDtos) {
    if (x instanceof ManagerAllocationDTO) {
         Manager m = (x(ManagerAllocationDTO)).getManager();
         // save manager etc to managerallocs
    } else if (x.getId() == AllocationDTO.TYPE_SPECIAL1) {
         // save to specialAlloc1 table
    } else if (x.getId() == AllocationDTO.TYPE_SPECIAL2) {
         // save to specialAlloc2 table
    }
}

The ManagerAllocationDTO has an extra field relating the allocation to a manager, but for the specialalloc1/2 cases I have not made a subtype because the only difference in the data is the table it is saved to.

My question is a bit of a soft design question - is this the best way to go about doing this?

Ben Page
  • 3,011
  • 4
  • 35
  • 37
  • 4
    Just a comment on your code: you really should use the ``equals`` method instead of ``==`` to compare your strings. Especially when playing with databases. – Vivien Barousse May 19 '11 at 11:44
  • 1
    Not too sure about code like "(x.getId() == "specialAlloc2")" - are you actually interested in object equality or should you really be using "x.getId().equals("specialAlloc2")" ? – DaveH May 19 '11 at 11:44
  • "specialAlloc2" is a constant, so i think object equality testing is ok in this case. I changed my code to reference said content right after I posted this and realised the same thing you two just commented. I have edited the question to reflect my changes. Do you agree? – Ben Page May 19 '11 at 12:02
  • 1
    See this similar question: http://stackoverflow.com/questions/5579309/switch-instanceof – Raedwald May 19 '11 at 12:19

1 Answers1

1

One way to seperate different instances, without instanceOf and if-else-cascade is to use the Visitor Design pattern.

  • new Interface: AllocationVisitor with one method for each concrete subclass of AllocationDTO:

    • visit(TYPE_SPCIAL1 dto)
    • visit(TYPE_SPCIAL2 dto)
  • AllocationDTO: abstract void acceptVisitor(AllocationVisitor visitor)

  • Each concreate subclass of AllocationDTO implements the acceptVisitor() this way: void acceptVisitor(AllocationVisitor visitor){visit(this);} //the correct visit method is choosen by compile time type.
  • Your DTO implements the AllocationVisitor Interface (with an inner class), create an instance of this, and passes it to the elements of listOfAllocationDtos.

DAO:

AllocationVisitor saveVisitor = new AllocationVisitor() {
  visit(TYPE_SPCIAL1 dto) {//what ever you need}
  visit(TYPE_SPCIAL2 dto) {//what ever TYPE_SPCIAL2 needs}
}
for (AllocationDTO x : listOfAllocationDtos) {
  x.visit(saveVisitor);
}
Ralph
  • 118,862
  • 56
  • 287
  • 383
  • This looks like I would have to add the saving logic to the DTO. I don't like this idea as the idea is the DTO is basically a data container that has no knowledge of how it is persisted/transmitted etc. The DAO should know about the DTO but not the other way around. Have I understood the visitor pattern correctly? – Ben Page May 19 '11 at 13:58
  • No, if you follow the pattern, then the DTO has only a method that is like a callback, and the logic belongs to the DAO – Ralph May 19 '11 at 15:17
  • @Ben Page: I have extended my answer to explain the Visitor pattern. This pattern is one of the most complex (and hard to understand once from the static structure of view) of the GOF book. But even if you do not use it for this case, you should have a look at his power full pattern. -- You can use it every where where you need a Case depending on a type. Then you will never forget to "extend the case" if you add an new Type, because the compiler will check it. – Ralph May 19 '11 at 15:31
  • Thank you for the explanation. I understand how it works and (hopefully) will be able to implement it. – Ben Page May 24 '11 at 09:53