Showing posts with label refactoring. Show all posts
Showing posts with label refactoring. Show all posts

Wednesday, October 17, 2012

How to eliminate switches and enums with simple polymorphism

Hi there! Today we will see, based on a real example, how we can banish ugly switch-cases and enums from the code with simple polymorphism. The example bellow has been written by my self a few months ago. At this time i was convinced that it was simple and good enough. Now i know it better and i'm convinced that it could be better, more flexible and maintainable with little effort but with huge effect. The following example handles states of buttons in a game field i'm programming.


UML




Interface EnableDisable



import java.util.List;

import javax.swing.JButton;

public interface EnableDisable {

    public void disableButtons(List<JButton> gameFieldButtons, int... indizes);

    public void disableButtons(JButton selectedButton, List<JButton> gameFieldButtons, int... steps);

    public void enableButtons(List<JButton> gameFieldButtons, int... indizes);

    public void enableButtons(JButton selectedButton, List<JButton> gameFieldButtons, int... steps);

}

Class EnableDisableUtil

import java.awt.Color;
import java.util.List;

import javax.swing.JButton;
import javax.swing.border.LineBorder;

public class EnableDisableUtil implements EnableDisable {

    public void disableButtons(List<JButton> gameFieldButtons, int... indizes) {
        disableOrEnable(Choice.DISABLE, gameFieldButtons, indizes);
    }

    public void disableButtons(JButton selectedButton, List<JButton> gameFieldButtons, int... steps) {
        disableOrEnable(Choice.DISABLE, selectedButton, gameFieldButtons, steps);
    }

    public void enableButtons(List<JButton> gameFieldButtons, int... indizes) {
        disableOrEnable(Choice.ENABLE, gameFieldButtons, indizes);
    }

    public void enableButtons(JButton selectedButton, List<JButton> gameFieldButtons, int... steps) {
        disableOrEnable(Choice.ENABLE, selectedButton, gameFieldButtons, steps);
    }

    private void disableOrEnable(Choice choice, List<JButton> gameFieldButtons, int... indizes) {
        switch (choice) {
        case ENABLE:
            for (int i = 0; i < indizes.length; i++) {
                enable(indizes[i], gameFieldButtons);
            }
            break;
        case DISABLE:
            for (int i = 0; i < indizes.length; i++) {
                disable(indizes[i], gameFieldButtons);
            }
            break;
        default:
            break;
        }

    }

    private void disableOrEnable(Choice choice, JButton selectedButton, List<JButton> gameFieldButtons, int... steps) {
        for (int i = 0; i < steps.length; i++) {
            int valueFromSelectedButton = Integer.valueOf(selectedButton.getText());
            int valueFromButtonToBeDisabled = steps[i] + valueFromSelectedButton;
            int indexFromButtonToBeDisabled = valueFromButtonToBeDisabled - 1;// because gameFieldButtons starts by 0
            switch (choice) {
            case ENABLE:
                enable(indexFromButtonToBeDisabled, gameFieldButtons);
                break;
            case DISABLE:
                disable(indexFromButtonToBeDisabled, gameFieldButtons);
                break;
            default:
                break;
            }
        }
    }

    private void enable(int buttonIndex, List<JButton> gameFieldButtons) {
        JButton buttonToBeDisabled = gameFieldButtons.get(buttonIndex);
        buttonToBeDisabled.setEnabled(true);
        buttonToBeDisabled.setBorder(new LineBorder(Color.red, 1, false));
    }

    private void disable(int buttonIndex, List<JButton> gameFieldButtons) {
        JButton buttonToBeDisabled = gameFieldButtons.get(buttonIndex);
        buttonToBeDisabled.setEnabled(false);
        buttonToBeDisabled.setBorder(new LineBorder(Color.lightGray, 1, false));
    }

    private enum Choice {
        ENABLE, DISABLE;
    }

}

What are we doing in this example?

Well, here i'm disabling or enabling some gameFields from my application depending on the method i call. (public methods). Now my question: What's wrong with that? It seems to be pragmatic for this simple use case, right? Well although at first glance this use case seems plausible and functionally ok(it passes the unit test), this design has potential for improvement. Let's point out first what i did "wrong":
  • The name of my interface is not smart. It implies more then one responsabilities.
  • I repeat myself in the signature of the methods of EnableDisable. Only the parameters are different
  • The enum and switches signals fix states to me. So it would be better to handle it over polymorphie.

Let's do the changes. First i wanna rename my interface from EnableDisable to Switchable and let's also eliminate 2 methods. In the sequence let's declare 2 classes called SwitchOn and SwitchOff.


Nice! Thats a lot more interesting. But let's take a look at the concrete classes SwitchOn and SwitchOff to be able to decide if it is good this way or not.

import java.util.List;
import javax.swing.JButton;

public interface Switchable {

    public void toSwitch(List<JButton> gameFieldButtons, int... indizes);

    public void toSwitch(JButton selectedButton, List<JButton> gameFieldButtons, int... steps);

} 


import java.awt.Color;
import java.util.List;
import javax.swing.JButton;
import javax.swing.border.LineBorder;

public class SwitchOn implements Switchable {

    @Override
    public void toSwitch(List<JButton> gameFieldButtons, int... indizes) {
        switchOn(gameFieldButtons, indizes);
    }

    @Override
    public void toSwitch(JButton selectedButton, List<JButton> gameFieldButtons, int... steps) {
        switchOn(selectedButton, gameFieldButtons, steps);
    }

    private void switchOn(JButton selectedButton, List<JButton> gameFieldButtons, int... steps) {
        for (int i = 0; i < steps.length; i++) {
            int valueFromSelectedButton = Integer.valueOf(selectedButton.getText());
            int valueFromButtonToBeDisabled = steps[i] + valueFromSelectedButton;
            int indexFromButtonToBeDisabled = valueFromButtonToBeDisabled - 1;// because gameFieldButtons starts by 0
            enable(indexFromButtonToBeDisabled, gameFieldButtons);
        }
    }

    private void switchOn(List<JButton> gameFieldButtons, int... indizes) {
        for (int i = 0; i < indizes.length; i++) {
            enable(indizes[i], gameFieldButtons);
        }
    }

    private void enable(int buttonIndex, List<JButton> gameFieldButtons) {
        JButton buttonToBeDisabled = gameFieldButtons.get(buttonIndex);
        buttonToBeDisabled.setEnabled(true);
        buttonToBeDisabled.setBorder(new LineBorder(Color.red, 1, false));
    }
} 


import java.awt.Color;
import java.util.List;
import javax.swing.JButton;
import javax.swing.border.LineBorder;

public class SwitchOff implements Switchable {

    @Override
    public void toSwitch(List<JButton> gameFieldButtons, int... indizes) {
        switchOff(gameFieldButtons, indizes);
    }

    @Override
    public void toSwitch(JButton selectedButton, List<JButton> gameFieldButtons, int... steps) {
        switchOff(selectedButton, gameFieldButtons, steps);
    }

    private void switchOff(JButton selectedButton, List<JButton> gameFieldButtons, int... steps) {
        for (int i = 0; i < steps.length; i++) {
            int valueFromSelectedButton = Integer.valueOf(selectedButton.getText());
            int valueFromButtonToBeDisabled = steps[i] + valueFromSelectedButton;
            int indexFromButtonToBeDisabled = valueFromButtonToBeDisabled - 1;// because gameFieldButtons starts by 0
            disable(indexFromButtonToBeDisabled, gameFieldButtons);
        }
    }

    private void switchOff(List<JButton> gameFieldButtons, int... indizes) {
        for (int i = 0; i < indizes.length; i++) {
            disable(indizes[i], gameFieldButtons);
        }
    }

    private void disable(int buttonIndex, List<JButton> gameFieldButtons) {
        JButton buttonToBeDisabled = gameFieldButtons.get(buttonIndex);
        buttonToBeDisabled.setEnabled(false);
        buttonToBeDisabled.setBorder(new LineBorder(Color.lightGray, 1, false));
    }
} 


Don't Repeat Yourself (DRY)

Well as we can see this is a lot better but we can still see a lot of dupplicated code inside of it. The only difference in the implementation from SwitchOn or SwitchOff is the setEnable(true) or setEnable(false) in the methods enable(...) or disable(...) and the LineBorder of both. Let's do one more elegant change to it like this:


import java.util.List;
import javax.swing.JButton;
import javax.swing.border.LineBorder;

public abstract class SwitchAbstract implements Switchable {
 @Override
 public void toSwitch(List<JButton> gameFieldButtons, int... indizes) {
  switchWithIndex(gameFieldButtons, indizes);
 }
 @Override
 public void toSwitch(JButton selectedButton, List<JButton> gameFieldButtons, int... steps) {
  switchWithSteps(selectedButton, gameFieldButtons, steps);
 }
 private void switchWithIndex(List<JButton> gameFieldButtons, int... indizes) {
  for (int i = 0; i < indizes.length; i++) {
   turnSwitchTo(indizes[i], gameFieldButtons);
  }
 }
 private void switchWithSteps(JButton selectedButton, List<JButton> gameFieldButtons, int... steps) {
  for (int i = 0; i < steps.length; i++) {
   int valueFromSelectedButton = Integer.valueOf(selectedButton.getText());
   int valueFromButtonToBeSwitched = steps[i] + valueFromSelectedButton;
   int indexFromButtonToBeSwitched = valueFromButtonToBeSwitched - 1;// because gameFieldButtons starts by 0
   turnSwitchTo(indexFromButtonToBeSwitched, gameFieldButtons);
  }
 }
 private void turnSwitchTo(int buttonIndex, List<JButton> gameFieldButtons) {
  JButton buttonToSwitch = gameFieldButtons.get(buttonIndex);
  buttonToSwitch.setEnabled(switchTo());
  buttonToSwitch.setBorder(howShallLineBorderLooksLike());
 }
 protected abstract boolean switchTo();
 protected abstract LineBorder howShallLineBorderLooksLike();
} 


public class SwitchOn extends SwitchAbstract {
 @Override
 protected boolean switchTo() {
  return Boolean.TRUE;
 }
 @Override
 protected LineBorder howShallLineBorderLooksLike() {
  return new LineBorder(Color.red, 1, false);
 }
} 


public class SwitchOff extends SwitchAbstract {
 @Override
 protected boolean switchTo() {
  return Boolean.FALSE;
 }
 @Override
 protected LineBorder howShallLineBorderLooksLike() {
  return new LineBorder(Color.lightGray, 1, false);
 }
} 

Why did i name the class SwitchAbstract and not AbstractSwitch?

Well maybe you did not noticed or it is not so obvious at first sight, but i has a good reason. the reason is in the way i'm able to search in my IDE. Naming my classes this way alls "Switch..." classes will be presented as a list in a very nice way while looking for the keyword "switch" in Eclipse. 



Results in facts:

When talking about clean code i ofen hear the argumet: It makes my code a lot more complex and generate a lot of useless code. Well let's see if this is true. I have more classes. For this case it must be automatically more complex and of course it must be have generated more lines of code right? 

Old version: Lines of code:  96 Lines of code 
New version: Lines of code: 73 Lines of code

Old class and interface names: Naming was not that simple understandable.
New class and interface names: Clear, does not implies more then one responsabilities

Old class implementation: Costs a lot and had ugly switch-cases and enums in it.
New class implemention: No effort, no enums, no ungly switche-cases  

😱👇 PROMOTIONAL DISCOUNT: BOOKS AND IPODS PRO ðŸ˜±ðŸ‘‡

Be sure to read, it will change your life!
Show your work by Austin Kleonhttps://amzn.to/34NVmwx

This book is a must read - it will put you in another level! (Expert)
Agile Software Development, Principles, Patterns, and Practiceshttps://amzn.to/30WQSm2

Write cleaner code and stand out!
Clean Code - A Handbook of Agile Software Craftsmanship: https://amzn.to/33RvaSv

This book is very practical, straightforward and to the point! Worth every penny!
Kotlin for Android App Development (Developer's Library): https://amzn.to/33VZ6gp

Needless to say, these are top right?
Apple AirPods Pro: https://amzn.to/2GOICxy

😱👆 PROMOTIONAL DISCOUNT: BOOKS AND IPODS PRO ðŸ˜±ðŸ‘†


Saturday, March 17, 2012

Making beans/data reusable and simultaneously reducing class size to only one method

Hi there! Today i wanna share an idea with you (generic bean/Data)

If there is one thing a hate is to write beans, getter and setters again, again and again. I was searching for a solution on how to make beans reusable and classes more readable, smaller and cleaner. A typical or common situation is when you need domain objects. I've caught myself constantly writting the "same  boring code" again and again. So i decided to search for a solution, which could help me saving time. I had an idea, which i would like to share with you. I've called it: "GenericBean".

IMPORTANT: After listening to the feedbacks from other developers I decided to let this post alive just to show to you, why this approach is not recommend and shall not be used. Stay with POJO's. 

Let's say we have a database table called "address". This address table may contain following attributes:
  • ID
  • FIRST_NAME
  • SECOND_NAME
  • STREET
  • HOUSE_NUMBER
  • ZIP_CODE
  • LAND
Let's say now, we need a domain class called: SimpleDomainAddress. The normal way would be to do something like this:

public class SimpleDomainAddress {
 
 private double addressId;
 private String firstName;
 private String secondName;
 private String street;
 private int houseNumber;
 private String zipCode;
 private String land;


 public double getAddressId() {
  return this.addressId;
 }

 public void setAddressId( double addressId ) {
  this.addressId = addressId;
 }

 public String getFirstName() {
  return this.firstName;
 }

 public void setFirstName( String firstName ) {
  this.firstName = firstName;
 }

 public String getSecondName() {
  return this.secondName;
 }

 public void setSecondName( String secondName ) {
  this.secondName = secondName;
 }

 public String getStreet() {
  return this.street;
 }

 public void setStreet( String street ) {

  this.street = street;
 }

 public int getHouseNumber() {
  return this.houseNumber;
 }

 public void setHouseNumber( int houseNumber ) {
  this.houseNumber = houseNumber;
 }

 public String getZipCode() {
  return this.zipCode;
 }

 public void setZipCode( String zipCode ) {
  this.zipCode = zipCode;
 }

 public String getLand() {
  return this.land;
 }

 public void setLand( String land ) {
  this.land = land;
 }
}

So this is the way everybody would do i think. Now if you have another domain object you'll do that again and so on generating a lot of unnescessary code lines in my point of view.

Let us hold on to what we do here:
  • We always define new variables
  • We always define new methods
  • We should have added "theoretically" some comments to it. 
The following UML diagram visualizes GenericBean approach:
 

Null Object
First of all we define a NULL object that could be written like this:

public class Null {
 // NullObject
}

GenericBean
Then we write the generic bean. This bean could be bigger then the example here. This is only a show case to visualize the idea behind it. I will intentionally violate some code conventions by writting variables very short and beginning with an underscore following by a number. I'll explain later why I am doing this way.

public class GenericBean<A, B, C, D, E, F, G, H, I, J> {
 private A _0;
 private B _1;
 private C _2;
 private D _3;
 private E _4;
 private F _5;
 private G _6;
 private H _7;
 private I _8;
 private J _9;

 public A get_0() {
  return this._0;
 }
 
 public void set_0( A _0 ) {
  this._0 = _0;
 }

 public B get_1() {
  return this._1;
 }

 public void set_1( B _1 ) {
  this._1 = _1;
 }

 public C get_2() {
  return this._2;
 }

 public void set_2( C _2 ) {
  this._2 = _2;
 }

 public D get_3() {
  return this._3;
 }

 public void set_3( D _3 ) {
  this._3 = _3;
 }

 public E get_4() {
  return this._4;
 }

 public void set_4( E _4 ) {
  this._4 = _4;
 }

 public F get_5() {
  return this._5;
 }

 public void set_5( F _5 ) {
  this._5 = _5;
 }

 public G get_6() {
  return this._6;
 }

 public void set_6( G _6 ) {
  this._6 = _6;
 }

 public H get_7() {
  return this._7;
 }

 public void set_7( H _7 ) {
  this._7 = _7;
 }

 public I get_8() {
  return this._8;
 }

 public void set_8( I _8 ) {
  this._8 = _8;
 }

 public J get_9() {
  return this._9;
 }

 public void set_9( J _9 ) {
  this._9 = _9;
 }
}

Shriking to only one method
Ok, at this point we have no profit of it right? Let's write know the same class SimpleDomainAddress again, but now using the GenericBean and Null Object.

public class SimpleDomainAddress {
 
 private GenericBean<Double, String, String, String, Integer, String, String, Null, Null, Null> genericBean;

 public SimpleDomainAddress() {
/** 
Database table: Id, Firstname, Secondname, Street, HouseNumber, Zipcode, Land
(The last 3 entries doesn't exit in the database, so we set our generic bean to Null)
*/
  this.genericBean = new GenericBean<Double, String, String, String, Integer, String, String, Null, Null, Null>();
 }

 /**
  * This generic bean represents the database table. 
  * See bellow how to get and set the values from it.
  * <ul>
  * <li>get_0 <b>return</b> AddressId</li>
  * <li>get_1 <b>return</b> Firstname</li>
  * <li>get_2 <b>return</b> Secondname</li>
  * <li>get_3 <b>return</b> Street</li>
  * <li>get_4 <b>return</b> HouseNumber</li>
  * <li>get_5 <b>return</b> Zipcode</li>
  * <li>get_6 <b>return</b> Land</li>
  * <li>get_7 Null (not used)</li>
  * <li>get_8 Null (not used)</li>
  * <li>get_9 Null (not used)</li>
  * </ul>
  */
 public GenericBean<Double, String, String, String, Integer, String, String, Null, Null, Null> getGenericBean() {
  return this.genericBean;
 }
}

Good side effects:
  • With this approach, the class is shrinking to less than a few lines of code.
  • A good side effect is the comment that is gaining in importance and now is no longer redundant.
  • If we notice that the database needs to be expanded, we only need to replace a NULL entry with the new value and we're done. 
  • Easy to learn (this example says all), simple, reusable.
  • We always know which entry is the first and the last in the database (get_1 & get_6 in this example)
The reason why i wrote the variables like "_0", _1" and so on is that this way, when i type "get" or "set" in my IDE, i'll get the methods in the same order(sequence) as i defined the database fields in my constructor as you can see above. This make the usage in association with the comment more intuitive and powerful.

😱👇 PROMOTIONAL DISCOUNT: BOOKS AND IPODS PRO ðŸ˜±ðŸ‘‡

Be sure to read, it will change your life!
Show your work by Austin Kleonhttps://amzn.to/34NVmwx

This book is a must read - it will put you in another level! (Expert)
Agile Software Development, Principles, Patterns, and Practiceshttps://amzn.to/30WQSm2

Write cleaner code and stand out!
Clean Code - A Handbook of Agile Software Craftsmanship: https://amzn.to/33RvaSv

This book is very practical, straightforward and to the point! Worth every penny!
Kotlin for Android App Development (Developer's Library): https://amzn.to/33VZ6gp

Needless to say, these are top right?
Apple AirPods Pro: https://amzn.to/2GOICxy

😱👆 PROMOTIONAL DISCOUNT: BOOKS AND IPODS PRO ðŸ˜±ðŸ‘†

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.

😱👇 PROMOTIONAL DISCOUNT: BOOKS AND IPODS PRO ðŸ˜±ðŸ‘‡

Be sure to read, it will change your life!
Show your work by Austin Kleonhttps://amzn.to/34NVmwx

This book is a must read - it will put you in another level! (Expert)
Agile Software Development, Principles, Patterns, and Practiceshttps://amzn.to/30WQSm2

Write cleaner code and stand out!
Clean Code - A Handbook of Agile Software Craftsmanship: https://amzn.to/33RvaSv

This book is very practical, straightforward and to the point! Worth every penny!
Kotlin for Android App Development (Developer's Library): https://amzn.to/33VZ6gp

Needless to say, these are top right?
Apple AirPods Pro: https://amzn.to/2GOICxy

😱👆 PROMOTIONAL DISCOUNT: BOOKS AND IPODS PRO ðŸ˜±ðŸ‘†