Refactoring instanceOf, moving logic to POJO when it has database interaction

Question

I'm refactoring some old code, I have a lot of istanceOf in the business part:

                if (record instanceof RecordA) {
                    RecordA recordA = (RecordA) record;
                    //mapping to recA and other stuff...
                    recA_dao.create(recA); //save a copy of the record to the db
                    checkRecord_A(recordA); //logic that use dao
                } else
                if (record instanceof RecordB) {
                    RecordB recordB = (RecordB) record;
                    //mapping and other...
                    recB_dao.create(recB);
                    checkRecord_B(recordB); //logic that use dao
                }//so on...

I'm trying to create an interface, and force the POJOs RecordA, RecordB,... to implement it (the Record don't have commons parts). The problem is that in this way I should move both the database operation and logic business to check the validity of the record (logic that sometimes use dao too...) to the POJOs.

Is it better to let the code as it is? Is it ok to move the logic and dao to the POJO or does it exist some design pattern that can help me to have a better design?

I was thinking too about creating a class with the logic and the dao calls, and call it from the methods that implements interface in pojos, but business and pojo are 2 separate project, and this would mean to include in the pojo app, the maven dependency to the business project, and this don't look good to me because it creates a circular dependency.

Any suggest?

Answer 1

This seems like textbook case for Visitor Pattern.

You can have a RecordValidationVisitor and RecordDaoVisitor that "visits" each type of record accordingly. So you can freely combine things that process records as you wish.

You will define your visitor like this:

interface RecordVisitor {
  void visitRecordA(value: RecordA)
  void visitRecordB(value: RecordB)
}

and your classes as:

class RecordA {
  void accept(visitor: RecordVisitor) {
    visitor.visitRecordA(this) 
  }
}

And then you can have following visitors:

class RecordDaoVisitor extends RecordVisitor {
  
  void visitRecordA(value: RecordA) {
    recA_dao.create(value);
  }

  void visitRecordB(value: RecordB) {
    recB_dao.create(value);
  }
}

class RecordDaoVisitor extends Visitor {
  // Implementation...
}

And finally you can use them like this:

var record = ??? // RecordA or RecordB doesn't matter

record.accept(recordDaoVisitor);
record.accept(recordValidationVisitor);

This has couple advantages over your current approach:

• Performance (instanceof checks are costly)
• When you add a new record type to the equation, your approach won't give a compile error about leaving out the new variant. If you add the visitRecordC to your RecordVisitor then you will get compile errors in bunch of places where you need to implement the handler for the new variant.