Follow treslines by email clicking Here!

Wednesday, February 29, 2012

How to eliminate If-Statements and InstanceOf with the Visitor-Pattern

Using the visitor pattern to eliminate If-Statements and InstanceOf


We all know the visitor pattern from the book GoF (gang of four) or from other famous books. The examples in there are usually associated with trees or composite structures. I was always searching for real life examples, where this pattern could be used, making software reusable and more readable. During a refactoring task, a collegue of mine had a really good idea, that i want to share with you.


Here is the code fragment from the method we want to refactore. The method has a lot of if-statements and instanceOf in there. It maps contacts to data transfer objects (DTO)


public static void mapKontakte(ContactType contactType, List<ErweiterteKontakt<? extends StringDomainEnum>> kontakte) {
        ch.abraxas.tax.register.registerimport.parser.ech0046.v10.ObjectFactory ech0046Factory =
                new ch.abraxas.tax.register.registerimport.parser.ech0046.v10.ObjectFactory();
        for (ErweiterteKontakt<? extends StringDomainEnum> kontakt : kontakte) {
            if (kontakt instanceof Telefon) {
                PhoneType phoneType = ech0046Factory.createPhoneType();
                phoneType.setPhoneCategory(new BigInteger( ((Telefon) kontakt).getKategorie().key()));
                phoneType.setPhoneNumber(kontakt.getDetail());
                contactType.getPhone().add(phoneType);
            } else if (kontakt instanceof Email) {
                EmailType emailType = ech0046Factory.createEmailType();
                emailType.setEmailCategory(new BigInteger( ((Email) kontakt).getKategorie().key()));
                emailType.setEmailAddress(kontakt.getDetail());
                contactType.getEmail().add(emailType);
            } else if (kontakt instanceof Internet) {
                InternetType internetType = ech0046Factory.createInternetType();
                internetType.setInternetCategory(new BigInteger( ((Internet) kontakt).getKategorie().key()));
                internetType.setInternetAddress(kontakt.getDetail());
                contactType.getInternet().add(internetType);
            }
            // Should we use an else branch to verify that we are mapping all Kontakte?
        }
    }


here is the same method after refactoring:


public static void mapKontakte(ContactType contactType,
   List<ErweiterteKontakt<? extends StringDomainEnum>> kontakte) {

  ErweiterterKontaktVisitor visitor = new BindingObjectFromKontaktVisitor(contactType);
  for (ErweiterteKontakt<? extends StringDomainEnum> kontakt : kontakte) {
   // calls the Kontakt to use the visitor
   kontakt.accept(visitor);
  }
 }


Well here is how to do it. In the abstract class ErweiterteKontakt we define following method:


public abstract void accept(ErweiterterKontaktVisitor visitor);


Then we implement this abstract method in the classes Email, Telefon and Internet. They all extends ErweiterkeKontakt.


 @Override
 public void accept(ErweiterterKontaktVisitor visitor) {
  visitor.visit(this);
 }

Defining the visitor interface


public interface ErweiterterKontaktVisitor {

 /**
  * Visit the telefon
  * 
  * @param telefon
  */
 public void visit(Telefon telefon);

 /**
  * visit the email
  * 
  * @param email
  */
 public void visit(Email email);

 /**
  * visit the internet
  * 
  * @param internet
  */
 public void visit(Internet internet);
}


Then Implementing the Visitor itself:


public class BindingObjectFromKontaktVisitor implements ErweiterterKontaktVisitor {

 private ch.abraxas.tax.register.registerimport.parser.ech0046.v10.ObjectFactory ech0046Factory = new ch.abraxas.tax.register.registerimport.parser.ech0046.v10.ObjectFactory();

 private ContactType contactType;

 /**
  * Konstruktor
  */
 public BindingObjectFromKontaktVisitor(ContactType contactType) {
  this.contactType = contactType;
 }

 /**
  * {@inheritDoc}
  */
 @Override
 public void visit(Telefon telefon) {
  PhoneType phoneType = ech0046Factory.createPhoneType();
  phoneType.setPhoneCategory(new BigInteger(telefon.getKategorie().key()));
  phoneType.setPhoneNumber(telefon.getDetail());
  contactType.getPhone().add(phoneType);
 }

 /**
  * {@inheritDoc}
  */
 @Override
 public void visit(Email email) {
  EmailType emailType = ech0046Factory.createEmailType();
  emailType.setEmailCategory(new BigInteger(email.getKategorie().key()));
  emailType.setEmailAddress(email.getDetail());
  contactType.getEmail().add(emailType);

 }

 /**
  * {@inheritDoc}
  */
 @Override
 public void visit(Internet internet) {
  InternetType internetType = ech0046Factory.createInternetType();
  internetType.setInternetCategory(new BigInteger((internet).getKategorie().key()));
  internetType.setInternetAddress(internet.getDetail());
  contactType.getInternet().add(internetType);

 }
}


Done!
No more If-Statements, no more InstanceOf and the code is now extandable and more flexible.


Advertising:
Optimized bets for playing EuroMillion's lottery on your Mobile Phone! 



How can i motivate other developers ?

Well i do that by talking about, i wear my treslines.com t-shirt, i wear my bracelets with the CCD (Clean Code Developer Initials and colors) and i use my brand new treslines bag. It feels fantastic and even on the train people start talking to me asking about it. It is also a great idea to motivate agile teams and to share your expirience. You may want a great gift to show your appreciation to your team. Surprise them with a gadget from www.treslines.com. I love it.


Other interesting blogs

How could we improve the quality of this article ?

If the content of this article does not help you, so please tell us how to improve the quality of it by giving your contructive feedbacks at the end of this blog. If it was useful to you giving and resuming to you the most important aspects of the subject treated, saving you a lot of time, then help us to maintain this blog with a little appreciation. With a small amount of your choice you help us to cover the prime costs like:
Hosting, autors’s research work, editorial work, blog quality, motivation to make things better than others resulting in a very useful information pool for you and a lot of other developers. Important : If you do not have the possibility to donate a little amount, than recommend this page to your friends. Thanks !







Source code convention tools

Literature, good books and references

How can i subscribe/feed this blog ?

Subscribe : click on the link at the end of this blog

Want to stay up to date ?

How can i rate this blog ?

Press google+1 once !

Where do i find more clean code knowledge and gadgets?

 


5 comments:

  1. A very interesting approach, very readable.

    ReplyDelete
  2. Thank you for this great article.
    But i have doubt.
    Why don't use visitor.visit(kontakt) directly?

    ReplyDelete
  3. Using visitor for this kind of multi dispatch is in my opinion an abomination, and as you can see your BindingObjectFromKontaktVisitor ends up being a messy big cludge of doing different things to different objects, all in the same place.

    If each of these objects get something generic "done" to them then they should preferably implement the interface with a method describing what is being done, then for each instance in the loop you just invoke the interface method. If this starts to get messy (ie too many interfaces or functionality too disparate from the original classes) then just have a decorator for each type that implements the interface and use a factory class to convert the different objects to the decorated classes, again each of which implement the interface, and again just invoke the method directory.

    Visitor pattern is for navigating object graphs and doing something to them. Using it for multiple dispatch is disgusting, and pattern overdosing for newbies.

    ReplyDelete
  4. I don't agree with your solution, because all your Visit Methods still belong to the same class. They should belong under each of the three case classes (Telefon / Email / Internet).

    ReplyDelete
  5. @thenonhacker normaly I would agree with you. A simple strategie pattern would be the cleaner and easier way. But the problem was, that we wanted to avoid coupling from Telefon, Email and Internet classes to the ContactType(which are generated classes).
    With the Visitor pattern we have the the advantage that this 3 Classes doesn't depends on the ConcatType and we can add new functionality with other visitorimplementations.

    ReplyDelete