Pages

Showing posts with label clean code. Show all posts
Showing posts with label clean code. Show all posts

Monday, December 17, 2018

Java 8 Refactoring Part 5: Improving enum with a BiFunction

This is the last refactoring that Victor Rentea did in the devoxx conference in London. He used a BiFunction to be able to provide more flexibility to an Enum. A very interesting refactor.



Thursday, December 13, 2018

Java 8 Refactoring Part 4: Composition Over Inheritance using the Loan Design Pattern

The Loan design pattern is a pattern that follows the principle of favouring composition over inheritance. It was explained in the Devoxx conference in London.



Wednesday, December 12, 2018

Java 8 Refactoring Part 3: Removing null checks and introducing Optional

The Optional feature of Java 8 it's very powerful it can prevent lot's of bugs and also make our code look more tidy. In this video I show an example which is very similar to the one presented at Devoxx London in 2018 by V. Rentea


Sunday, December 9, 2018

Java 8 Refactoring Part 2: Stream Wrecks

A stream wreck is a complex concatenation of streams that tries to provide some results in a one-liner. One-liners are good but concatenation of Streams is not good since makes the code quite difficult to follow. To fix stream wrecks we need to extract methods, local variables or classes in order to be able to improve the readability of the code.


Saturday, December 8, 2018

Java 8 Refactoring Part 1: Extracting Anonymous function to it's own class

In this video I mimic the first of the design patterns presented by Victor Rentea in the Devoxx conference in London in 2018.


Perhaps worth mentioning that the advantages of extracting anonymous functions into separate classes are:

- better readability of the code
- code easier to test
- more maintainable code

Thursday, April 13, 2017

Implementing Strategy Design Pattern(Practical example)

In this video I show how to refactor some ugly if-else nested statements using polymorphism through the strategy design pattern.

Please note that I didn't use code coverage in this occasion, this was more of an Spike than a refactor I would say. My goal was to just show the implementation of the pattern and make a brief video. Of course if you are trying this in real software, please make sure that your code is covered by unit tests before proceeding.




Tuesday, April 11, 2017

Refactor switch to Enum

Do switch statements sound good to you? 
Trust me better switch to Enum before it destroys your ears ;)



Friday, April 7, 2017

Refactoring "Feature Envy" Code Smell

In this video I explain the "Feature Envy" Code Smell and how it can be refactored. Sorry I made a mistake in the screen casts and I just selected the window rather than the area, you can't see the pop up menus and auto suggest when I press Ctrl + Space. Sorry about that :(( It takes me some time to make videos but if I get the chance soon I will repeat the demo and re-upload it.


Thursday, April 6, 2017

Dealing with Hardwired dependencies part 2 - Making a design decission

In this second video, I make a design decision and I remove the temporary refactor that I did earlier.
The Seam was not a bad idea, but it was a temporary solution only until I make a decision about the design of the class.


Tuesday, September 29, 2015

Discovering transitive dependencies in maven

Sometimes we discover a dependency in the project that we didn't put there. The reason for this is often a transitive dependency(a dependency of our dependency).
Keeping an eye on transitive dependencies and tidying up the pom files time after time is a good practice.
A way of discovering transitive dependencies is to use the command "mvn dependency:analyse".

This command comes from a set of tools inside the maven-dependency-plugin.
In order to use it, you have to include the dependency plugin in your project like this:


 <!- If you are just using it in a single module project->  
     <plugins>  
       <plugin>  
         <groupId>org.apache.maven.plugins</groupId>  
         <artifactId>maven-dependency-plugin</artifactId>  
         <version>2.10</version>  
       </plugin>  
     </plugins>  

  <!-- If you are using a multi module project then use this in the parent POM -->  
   <pluginManagement>  
    <plugins>  
     <plugin>  
      <groupId>org.apache.maven.plugins</groupId>  
      <artifactId>maven-dependency-plugin</artifactId>  
      <version>2.10</version>  
     </plugin>  
    </plugins>  
   </pluginManagement>  

This plugin can do multiple things, the analyze command is very useful for finding more information about dependencies.

Unused declared dependencies found: The dependencies are not used in the project
Used undeclared dependencies found: Transitive dependencies used in the project(not declared in the pom)

The output of the command will be presented as warning in the terminal, if you want to be very strict about keeping your your pom tidy,
you could add the analyze command in the build lifecycle and configure maven to fail the build if any warnings are found(Maybe I explain that in another
post sometime ;) ).

Another great tool that comes in the dependency plugin and that its worth mentioning, is
 "mvn dependency:tree".
It will help you visualise the dependencies tree of your pom and understand it better.

Just some more interesting stuff about maven if you want to read more:
http://blog.florian-hopf.de/2014/01/analyze-your-maven-project-dependencies.html
http://www.kyleblaney.com/maven-best-practices/
https://maven.apache.org/plugins/maven-dependency-plugin/

Wednesday, October 29, 2014

Indaface!



Recently my team leader gave me an S.L.A.P, hehehe...
Don't worry this is not a case of bullying, it just a way of refering to an important Object Oriented Principle known as the Single Level of Abstraction Principle.
As Usual at the office paring on an story, when we got to the refactoring bit, I was told to improve a method that had some ugly conditional logic on it, and also some duplication.
Instead of removing the duplication, I just delegated all the problematic part to another method, so the original method would look smaller and cleaner, but...
He said to me: Do you think that code you just delegated is located now at a different level of abstraction?... Indaface! Violation of S.L.A.P
Below, an example that somehow recreates todays funny situation :P Sorry, can't show you the real code(Dont wanna get in trouble :) ).

 /*  
     We have a vault that its being populated and categorized but there is a little 
     duplication issue when populating the vault with CAT-3 items. Items withGrook() 
     are definitely CAT-3, but those withTrook() are only categorized as CAT-3 if 
     klop.isHigh().   
 */  
    public Vault stuff(Klop klop, Vault vault) {  
     vault.include("CAT-1", azra());  
     vault.include("CAT-2", khy());  
     vault.include("CAT-3", things.stream().filter(withGrook()).
     map(toSponge()).map(toPlik(klop)).collect(toList()));  
     if(klop.isHigh())  
       vault.include("CAT-3", things.stream().filter(withTrook()).
       map(toSponge()).map(toPlik(klop)).collect(toList()));  
     return vault;  
     }  
 /*  
     At the beggining we may feel tempted to extract that vault.include, to its own
     method which will be specific to CAT-3 items, regardles that the stuff method looks
     shorter, the problem is that we did not remove the duplication plus we are disrespecting
     the S.L.A.P principle.  
 */      
      public Vault stuff(Klop klop, Vault vault) {  
           vault.include("CAT-1", azra());  
           vault.include("CAT-2", khy());  
           includeCat3Items(vault,klop);  
           return vault;  
        }  

   private void includeCat3Items(Vault vault, Klop klop) {  
           if(klop.isHigh())  
             vault.include("CAT-3", things.stream().filter(withGrook()).
             map(toSponge()).map(toPlik(klop)).collect(toList()));  
             vault.include("CAT-3", things.stream().filter(withTrook()).
             map(toSponge()).map(toPlik(klop)).collect(toList()));  
   }  
  
  /*If we want to respect S.L.A.P, in this case what we need to extract, is just the 
    changing part, and delegate the condition to check if klop.isHigh, to the delegate
     method.  
  */  
     public Vault stuff(Klop klop, Vault vault) {  
     vault.include("CAT-1", azra());  
     vault.include("CAT-2", khy());  
     vault.include("CAT-3", things.stream().filter(ook(klop)).
     map(toSponge()).map(toPlik(klop)).collect(toList()));  
     return vault;  
   }  

   private Predicate<String> ook(Klop klop) {  
     return klop.isHigh() ? withGrook(): withTrook();  
   }  

 /*  
     The final refactor could even go one step further by extracting implementation 
     detail into a plu(Klop klop) method  
 */      
   public Vault stuff(Klop klop, Vault vault) {  
     vault.include("CAT-1", azra());  
     vault.include("CAT-2", khy());  
     vault.include("CAT-3", plu(klop));  
     return vault;  
   }
  
   private List<Object> plu(Klop klop) {  
     return things.stream().filter(ook(klop)).map(toSponge()).map(toPlik(klop)).collect(toList());  
   }  
   private Predicate<String> ook(Klop klop) {  
     return klop.isHigh() ? withGrook(): withTrook();  
   }  


This post is dedicated to L.K, thanks for the patience :)

Tuesday, September 23, 2014

The Add-Delete ratio as a metric of code quality

Recently after reading some articles about software quality metrics, I started thinking a lot about what would be for me a metric that really could reflect the quality of the code in the system I am working in.

I am not a big fan of numbers but in the same way of an altimeter is a useful thing you want to look at when you are driving a plane, a good code quality metric could give you warning signs about system that you are building.

In page 15 of the book "Clean Code" by R.C Martin, it says that the only valid measure of code quality is, "What The Fucks per second".



Couldn't agree more, but the only problem with this metric is that is not easy to reflect in a chart so all others can see it and therefore have an idea of how bad the system really is...

Time ago while talking to a colleague here in London, we came to the conclusion that a really good metric to reflect the quality of the code we write, is the "Added-Removed code ratio".

So this is basically the ratio of added to removed lines of code in your system. 
To measure this, what you do is:

1 - At a point in time, take your system as a whole and count the amount of lines of code, of the target language, lets say Java(Just java, dont take xmls and other configuration files...).

2 - At a future point in time, take again the same system, and count the lines again.

3 - Write down the ratios and take note of the percentages the amounts of line are increasing, or decreasing and present the data in some chart.

 This metric if tracked often, such as the altimeter, will warn your department about when you need to invest more time in technical debt, and when your system is sustainable and you can keep pushing features.

I guess you wonder, how do I know this is right? 
Well, it is very simple, we just need to understand the definition of re-factoring which is:

"a change made to the internal structure of software to make it easier to understand and cheaper to modify without changing its observable behavior."

Re-factoring its very important, because the more understandable the system is, the better quality it will have. So it is not about just the smaller your code is, but about tracking the amount of effort you expend in adding and removing(specially removing) stuff from it.
- In a system where we see that the remove ratio numbers across time are going in a down trend(smaller gap between added and removed lines), it means that the developers are taking care and the overall  quality is improving.

- In a system where we see that the remove ratio numbers across time are going in an up trend(big gap between added and removed lines), it means that the teams are not investing enough time in improving the quality of the software they make.

I hope you liked the post, I appreciate feedback, this is just my opinion.



Tuesday, September 24, 2013

Keep calm and S.O.L.I.D

Last winter I had the chance to attend a talk by Robert.C.Martin(aka Uncle Bob) in Dublin. I think it was awesome. Uncle Bob is the author of Clean Code(I've got it signed by him in person :p) and also is a very influential person in the software industry.

I've been thinking to write something about some of the contributions he did to the industry and post it here on my blog since that talk.

One of the last things he said to us, was to read as much as we could...
Before starting to write this post I thought about those words for a while...
I think that it was a great advice.
Nobody negates that real experience is probably best, but reading books is also very important.
They carry the experiences of persons who were there before and also the rules and the principles those persons discovered and documented on their way.
Don't get me wrong, following rules and principles strictly does not always guarantee success("the world keeps changing") but understanding them can be of great help when facing great challenges.


So finally I decided to write this post about one of the biggest contributions of Uncle Bob to the world of object oriented programming, and that is the S.O.L.I.D principles.

Principle #1 
Single Responsibility Principle
If we have a class that has multiple responsibilities/features/reasons to change; Modifications done to it carry the risk of affecting other parts of the class(other responsibilities/features/reasons to change). 
In other words: Classes that do more than one thing are difficult to maintain. 


SRP says: "A class should have only one reason to change".
Lets have a look at an example:
 public class BMI {  
     //Stuff...  
      public void calculate(int heightInCm,int weightInGrms) {  
      //Implementation...  
     }  
     public void saveResults() {  
      //Implementation...  
     }   
     public TrainingPlan getTrainingPlan() {  
      //Implementation...  
     }  
 }  
As you can see, it is a clear example of a class with multiple reasons to change:

  •  Future business requirements might involve us changing the calculate method(e.g different metrics)
  •        Future business requirements might involve us changing the way the results are saved.
  •        Future business requirements might involve us changing the way the training plan is created based in the BMI.

We don't want all the above mentioned requirement changes to affect the class. If we don't seek for modularity at an early stage of development, we will very easily end up with difficult to maintain software.

What we need to do is think just on the one unique goal that the class will have.
Also we can think in what the class definitely will not do, so we can distiguish the other reasons to change that should not be there. Follow this way of thinking when fixing a violation of the SRP and it will help you detect the classes that you need to extract:

 public class BMICalculator {  
     //Stuff...  
         public void calculate(int heightInCm,int weightInGrms) {  
      //Implementation...  
     }  
     //Other methods that support the main goal of the class...  
 }  
 public class BMIToStorage {  
     //Stuff...  
         public void save(BMIResults results) {  
      //Implementation...  
     }  
     //Other methods that support the main goal of the class...  
 }  
 public class TrainingPlanCreator {  
     //Stuff...  
         public TrainingPlan getTrainingPlan(BMIResults results) {  
      //Implementation...  
     }  
     //Other methods that support the main goal of the class...  
 }  



Principle #2 
Open Close Principle

The motivation behind the Open Close principle is to extend/change behaviour without modifying the existing code. This principle says:
"modules should be open for extension but close for modification".
You probably think, that this sounds very contradictory, but in many OO programming languages like Java, there are mechanisms that will allow you to do this. 

One of those mechanisms is polymorphism. By defining abstract functions/methods

Let's have a look first at a violation of the open closed principle:

 public class Chef {  
      public void prepareMeal(Meal meal) {  
           if(meal.type.equals("veg")) {  
                prepareVeg();  
           }  
           else if(meal.type.equals("nonVeg")){  
                prepareNonVeg();  
           }  
      }  
      private void prepareVeg() {  
           //Implementation...  
      }  
      private void prepareNonVeg() {  
           //Implementation...  
      }  
 }  
 public class Meal {  
      String type;  
 }  
 public class NonVeg extends Meal {  
      public NonVeg() {  
           type = "nonVeg";  
      }  
 }  
 public class Veg extends Meal {  
      public Veg() {  
           type = "veg";  
      }  
 }  

In the above code, if a new requirement arrives saying to make some other type of meal different than veg and non veg, the class will need to be modified. The given above example is not maintainability friendly.

Let's see how to use polymorphism to remove that conditional logic and improve the solution:
 public class Chef {  
      public void prepareMeal(Meal meal) {  
           meal.cook();  
      }  
 }  
 public abstract class Meal {  
      public abstract void cook();  
 }  
 public class NonVeg extends Meal {  
      public void cook() {  
           //...  
      }  
 }  
 public class Veg extends Meal {  
      public void cook() {  
           //...  
      }  
 }  
As you can see the solution is more flexible, now it is easier to maintain and also we got rid of an evil flag that at long term will cause only problems when manipulating it. The class Meal that contains sensitive methods is open for extension but closed for modification.

The OCP principle is very powerful but we also must have in mind that by adding levels of abstraction(as alternative you can also think about Composition versus inheritance), we also increase the complexity and it is very important to understand that this principle should be applied only in those places where there is more likely to be often requirement changes. 

Principle #3 
Liskov substitution principle
This principle is concern about sub-classes classes replacing the behavior of their base class. 
If this occurs, the new classes can produce undesired effects when they are used/called in other parts of the program.

Liskov's Substitution Principle states that if a client is calling a base class, then the reference to the base class should be able to be replaced with a derived class without affecting the functionality of base class.

Lets have a look at an example of violation of this principle:

 public class Duck {  
      public void quack(){  
           //..            
      };  
      public void swim(){  
           //..  
    };  
 }  

Also a wild duck can quack and swim.
But what about Duck toys?

 public class DuckToy extends Duck {  
     private boolean batteriesIncluded;       
      public DuckToy(boolean batteriesIncluded) {  
           //...  
      }  
      public void swim() {  
           //...(This logic depends on the batteries)  
      }  
      public void playSound() {  
           //...  
      }  
 }  

As you see, some duck toys require batteries and also they don't really quack, they just play a sound. Even if there are no compilation errors and it looks tempting to include duck toy in this inheritance chain, this is clear violation of Liskov's substitution principle.
The reason is that if a client instantiates the base class, the derived class DuckToy, is not capable of replace it because the functionality is being affected.

 public class Pond {  
      public static void main(String[] args) {  
           Duck wildDuck = new WildDuck();  
           Duck duckToy = new DuckToy(true);  
           wildDuck.quack();  
           //This should not quack  
           duckToy.quack();  
           //Compilation error, real ducks don't play sounds or use batteries  
           //duckToy.playSound();  
      }  
 }  

One solution in this case could be to have a separate class by its own, to represent the duck toy.

 public class DuckToy {  
     private boolean batteriesIncluded;       
      public DuckToy(boolean batteriesIncluded) {  
           //...  
      }  
      public void swim() {  
           //...(This logic depends on the batteries)  
      }  
     public void quack() {  
           //Duck Toys don't quack  
      }  
      public void playSound() {  
           //...  
      }  
 }  

Principle #4 
Interface segregation principle

This is a very simple to understand principle, it says that clients should not be forced to implement interfaces they don't use Just that simple. Have a look at a violation of this principle:


 public interface Animal() {  
    public void fly();  
    public void run();  
    public void swim();  
 }  
 public class Dog implements Animal {  
    public void fly() {  
    //This is empty because dogs can't fly  
    }  
    public void run() {  
     //Implementation for running  
    }  
    public void swim() {  
     //Implementation for swimming  
    }  
 }  

That was horrible uh? So there are many ways you can avoid this.
One example could be to combine specific interfaces as per needed:

 public interface Runner() {  
    public void run();  
 }  
 public interface Swimmer() {  
    public void swim();  
 }  
 public interface Flyer() {  
    public void fly();  
 }  
 public class Dog implements Swimmer,Runner{  
    public void swim(){}  
    public void run(){}  
 }  
 public class Seagull implements Swimmer,Flyer{  
     public void swim(){}    
      public void fly(){}  
 }  

Degresion: While writing this example I just remembered a post that I wrote time ago about the use of Marker interfaces, that is also another interesting way of getting flexibility using interfaces. 

Principle #5 
Dependency inversion principle



This principle says "Don't depend on anything concrete, depend only on things that are abstract." So make sure that all of your dependencies point at things that are abstract.
This will bring safety to your code and also make it flexible.
Probably you are thinking that this principle, can be a bit radical; but obiously, in real following it always strictly is just very difficult(maybe even impossible).
A tip that you can use to verify that you are following this principle when you call a function, is to program to the interface and not the realization.

One great example of this principle in practice is the Template design pattern. Lets have a look first at a common violation of the principle:

 public class PizzaMaker {  
   public Pizza makeMeatPizza() {  
      Pizza pizza = new Pizza();  
     pizza.setBase(true);  
     pizza.setCheese(true);  
     pizza.setOregano(true);  
     pizza.setTomato(true);  
     pizza.setMeat(true);  
     cook(pizza);  
   }  
   public Pizza makeVeggiePizza() {  
      Pizza pizza = new Pizza();  
     pizza.setBase(true);  
     pizza.setCheese(true);  
     pizza.setOregano(true);  
     pizza.setTomato(true);  
     pizza.setMeat(false);  
     pizza.setVegetables(true);  
     cook(pizza);  
   }  
   private Pizza cook(Pizza pizza) {  
   //...  
   }  
 }  

The above example is a badly coded class that makes 2 types of pizzas... There are many bad things in this piece of code, but I will just focus on the violation of the principle we are discussing.
Since every call done to the pizza object is to a concrete method, what we get is something very rigid and inflexible.
Every pizza has some ingredients that are mandatory, such as the base, tomato, cheese and oregano,but the rest are optional, so: why do we care about making calls to concrete methods, to set those extra ingredients, if is not even our concern?

In the following snippet of code, a template method is introduced to abstract the optional part and let sub-classes implement them.

 public abstract class PizzaMaker {  
  //A method that has a call to an unimplemented abstract function,   
  //is known as a template method  
   public Pizza make() {  
     Pizza pizza = addBasicIngredients();  
     addSpecificIngredients(pizza);  
     cook(pizza);  
     return pizza;  
   }  
   public abstract void addSpecificIngredients(Pizza pizza);  
   private Pizza addBasicIngredients() {  
     Pizza pizza = new Pizza();  
     pizza.setBase(true);  
     pizza.setCheese(true);  
     pizza.setOregano(true);  
     pizza.setTomato(true);  
     return pizza;  
   }  
   private void cook(Pizza pizza) {  
     //...  
   }  
 }  
 public class MeatPizzaMaker extends PizzaMaker {  
   @Override  
   public void addSpecificIngredients(Pizza pizza) {  
     pizza.setMeat(true);  
     pizza.setVegetables(false);  
   }  
 }    
 public class VegetablesPizzaMaker extends PizzaMaker {  
   @Override  
   public void addSpecificIngredients(Pizza pizza) {  
     pizza.setMeat(false);  
     pizza.setVegetables(true);  
   }  
 }  

The S.O.L.I.D principles, were identified by Robert C.Martin, but the Acronym was created by Michael Feathers in the year 2000, today they are well known in the world of object oriented sofwtare and many there is plenty literature on books and internet about them.

Just for the end of this post I would like to share with you a great podcast interview that I found on the web, were Uncle Bob, explains S.O.L.I.D in detail.



Monday, May 27, 2013

Duplication - A bit between your teeth

I presume that pretty much everybody who has been in contact with the java programming language either in studies or professionally even for a little bit, knows that duplication is a code smell.
We all know that some consequences of duplication are: less maintainability, less readability, the code is more likely to contain bugs, performance issues... and many more.

I think there should not be need to explain what duplication is, but just in case let's write only one line to define it and also make sure that we understand it:
"Duplication is the existence of multiple copies of a single concept"


Are there any doubts until this point?...
Please go back and read once more the definition if you still are not sure about it.

So, how do we fix duplication?


But if it is so easy to understand what duplication is and also how simple it is to refactor, why do we find it all over the place, no matter what software we are looking at.

Well... there are many reasons why there is duplication all over the place:
  • sometimes we just rush to get things done and we don't care about it. 
  • maybe we think that refactoring is boring or is not profitable, so we have no patience to do it.
  • other times we think there are always more important things to do and duplication is not really affecting us.
  •  also sometimes when we want to do something about it, we just can't see it even if it is in front of us.             

 So, what do you think?... Any of this may be the reason why you are not fixing duplication?
If your reason is any of the first 3, I am sorry, but this blogpost will not be of any use to you and the best you can do is close this browser tab or surf somewhere else.

But if it is the 4th, the reason why you are not fixing the duplication, then you might find the following lines very interesting....

This is the secret why many programmers sometimes find it so difficult to deal with duplication...

There are different kinds of duplication, this are some of the most important:
  • Literal
  • Structural
  • Semantic

Literal duplication
This is a very easy to spot kind of duplication. It is basically literal values that are repeated in our code.

e.g
 public class RepeaterSpecification {  
      @Test  
      public void repeatsInput() {  
               assertThat("value",is(repeat("value")));  
      }  
      @Test  
      public void repeatsEmptyInput() {  
              assertThat("",is(repeat("")));  
      }  
     //...  
 }  

The most common way to fix this type of duplication is to create a local variable. Let's have a look at a possible refactor:

 public class RepeaterSpecification {  
      @Test  
      public void repeatsInput() {  
         String input = "value";  
         assertThat(input,is(repeat(input)));  
      }  
      @Test  
      public void repeatsEmptyInput() {  
         String input = "";  
         assertThat(input,is(repeat(input)));  
      }  
     //...  
 }  
It looks like we no longer have literal duplication. But unfortunately as you probably noticed, sometimes when fixing one type of duplication we generate another type of duplication. Keep reading to find out more...

Structural duplication
We can recognize this situation when the logic is duplicated but it operates on different data.
See this situation in the following example:

e.g
 public class RepeaterSpecification {  
      @Test  
      public void repeatsInput() {  
         String input = "value";  
         assertThat(input,is(repeat(input)));  
      }  
      @Test  
      public void repeatsEmptyInput() {  
         String input = "";  
         assertThat(input,is(repeat(input)));  
      }  
     //...  
 }  


As you can see I toke this code from the previous example. The assertThat() operation is identical, the only difference is the data that it uses. Let's have a little look at a way how to fix structural duplication:


public class RepeaterSpecification {  
      
      @Test  
      public void repeatsInput() {  
         assertThatInputIsRepeated("value");  
      }  

      @Test  
      public void repeatsEmptyInput() {  
         assertThatInputIsRepeated("");  
      }
 
      private void assertThatInputIsRepeated(String input) {  
            assertThat(input,is(repeat(input)));
      }
     //...  
 }


Now that we extracted the method that does the assertion we no longer have duplication.

Semantic duplication
This is  the situation where different code implementations represent the same functionality or concept.
The definition of semantic duplication teaches us that duplication can be invisible from the point of view of the code("The definition from the start of this post makes a way more sense now, uh? :)" ). This kind of duplication is probably one of the most difficult to spot.

e.g
public class TeamValidator {       
  
      public boolean isThereALeader(List<Members> team) {  
           Iterator<Member> iterator = team.getIterator();
           while(iterator.hasNext()) {
              Member member = iterator.next();
              String role = member.getRole();
              if(role.equals("Leader"))
                return true;
           }  
           return false;
      }  

      public boolean areThereAtLeast2NewJoiners(List<Members> team) {  
            for(Member member:team) {
                DateTime aMonthAgo = DateTime.now().minusMonths(1);
                if(member.startingDate().isAfter(aMonthAgo))
                return true; 
            }
            return false;   
      }
     //...  
 }


At first glance, when we look at the two methods in the code above we cannot really see much duplication. No matter how much we look at it, we will not see it. Semantic duplication is invisible from the implementation point of view. To spot it what we need to do is spot the behavioral anti pattern that is hidden in the code. If we think about it, basically the repetition going on is that both methods iterate one list of the same type and then apply certain criteria(Completely different implementations but one same concept).
Semantic duplication is more difficult to fix than other types of duplication and often requires a bigger refactoring effort. Here is one possible solution that helps us get rid of it using Java generics:

 public class TeamValidator {      
    public boolean isThereALeader(List<Member> team) {   
         return new LeaderVerifier<Member>().evaluate(team);  
    }   
    public boolean areThereAtLeast2NewJoiners(List<Member> team) {   
         return new NewJoinersVerifier<Member>().evaluate(team);  
    }  
 }  


 public abstract class LoopEvaluator<T> {

 public boolean evaluate(List<T> list) {
  for (T element : list) {
   if(evaluateElement(element)) {
    return true;
   }
  }
  return false;
 }
 
 public abstract boolean evaluateElement(T element);
 
}


public class LeaderVerifier<T extends Member> extends LoopEvaluator<T> {

 @Override
 public boolean evaluateElement(T element) {
  return element.getRole().equals("leader");
 } 
}



public class NewJoinersVerifier<T extends Member> extends LoopEvaluator<T> {

 private int newJoiners;
 
 @Override
 public boolean evaluate(List<T> list) { 
  this.newJoiners = 0;
  return super.evaluate(list);
 }
 
 @Override
 public boolean evaluateElement(T element) {
  DateTime aMonthAgo = DateTime.now().minusMonths(1);
  if(element.startingDate().isAfter(aMonthAgo)) 
   this.newJoiners++;
  return newJoiners == 2;
 }
}


Duplication is not a little topic at all. The 3 types of duplication I mentioned in this post are from my point of view some of the most common but there are many more. Here a link where I found where at the bottom you can find a mention to other kinds of duplication: http://blogs.agilefaqs.com/tag/code-smells/


                                                                     
 I am the spaghetti monster, You cant get rid of me!!!! Hahahaha......

Just to conclude this post, I want to say that I have the impression that duplication many times is an  underestimated code smell that undetected grows and grows and ends up transforming systems into spaghetti monsters.

Lets get rid of it while it is just a bit between your teeth! ;)







Friday, March 22, 2013

How do private methods affect our software testability?

 How many times did we hear the question:
“Should we test private methods?”
In this post I want to share with you my thoughts about this question and how I think we should proceed when having this doubt.
Methods with the access modifier private are code as the rest of the application, so this mean that they can potentially hide bugs.  So after a first impression my answer to that question is: “Yes, we should!”.  
Wait, a second… before you get mad at me. I want to rephrase my statement.
I think that question is completely not in place.  Instead of asking “Should we test private methods?”, we should ask: “How do private methods affect our software testability?”
Let’s consider different approaches to deal with private methods and see their pros and cons from the testability point of view:

Scenario 1 – Ok,ok you leave me no choice but reflection
This is what the official Java tutorials say about reflection: 
“Reflection is commonly used by programs which require the ability to examine or modify the runtime behavior of applications running in the Java virtual machine. This is a relatively advanced feature and should be used only by developers who have a strong grasp of the fundamentals of the language. With that caveat in mind, reflection is a powerful technique and can enable applications to perform operations which would otherwise be impossible.”
Ok, let’s  give it a try, imagine we have some private method, such as this one:



Hhmmm… So how to do this? Oh my god, do I really have to use reflection? Are you sure?
Let’s investigate a bit before making any decision.
Weeks ago I spent one full afternoon searching the internet looking for testability assurance pattern to test this in java. The only interesting discovery I did was that C# developers have a pattern for this called  “Internals Visible To Attribute”.  Unfortunately (Or maybe fortunately hehe…) that technique is not compatible with java.
There has to be an alternative, why can’t we just change the access modifier and test it normally?...
The common answer you get, when you ask that is:  You will compromise security, you will break encapsulation, you are lazy…
Ok,ok,you leave me no choice but reflection:

 Please don’t do this at your workplace J
Let’s see the Pros and Cons now.

Pros:
- The test is green and I tested the piece of code that I wanted to test
- I did not compromise the security of my software(:P Whatever….)
- I am an expert because the official java tutorials say that only cool dudes like me can use reflection.
Cons:
- A simple rename done by anybody to the method, return type or parameter will break the test and I will have to come back to rewrite the test. This type of test is very difficult to maintain.
- I broke the Security of my software. What am I talking about??? How is that possible if the method still private in the production code?? Hacking the internals of a class is a security.
- See how long that test is. Imagine you have to do this for a longer and more complex method.
More drawbacks(Taken from Java official tutorials website):



Scenario 2 – There is always a way out.
As mentioned in the previous scenario a design pattern that can help us testing private fields does not exist. But there are always ways out. Here I am going to suggest 2 and explain them:

- Protect your method: Just change the access modifier to protected. This will allow you to stub the method call and test using a pattern called Subclass & Override. Basically on your test package, you will be creating a fake that extends the class under test and then override using your expectations. You will then use that fake on your test.
This is a very common breaking dependency technique, since that private method will for sure be called somewhere else within the class and present a dependency inside other method/s(Must be broken in order to increase testability).
I don’t agree with this being a security risk because various reasons, one of them is that probably that method should not be private or, is too long to be tested indirectly, or probably that method should not even be there.
The only disadvantage I see is a decrease in the quality of the design, but I will justify that as the price we need to pay when we build software without using TDD or having testability in mind.

- Extract until you drop: Robert C. Martin in his book “Clean Code”, describes a refactoring methodology called “Extract until you drop”, the idea is refactor the code as much as possible until the point there the methods have not more than 4 lines of code. He says that a line with just a curly braces also count as a line.  If you think about it, your methods would be able to use an “if else” statement of just one line in each of the clauses.
If we want to unit test, we need to have units.  After doing this exhaustively on the method or class under test, we will probably see how that method was probably refactored into a new class, or maybe became so small that it can be tested indirectly via another method with no risk at all, since there are no longer inflection points(Places where logic of the software can potentially take a different path).
The main disadvantages of this approach will probably be that doing it probably would take too long, also on our way and specially if we are dealing with legacy code, we will need to use a lot of breaking dependency techniques and this will present a learning curve that will push us to break some of the design rules we were highly tight too in the past when the system was initially built.  But the benefit will be visible at short, mid and long term.

If you are concern about testability and you believe that testability is the path to quality in modern software, then the right question is How do private methods affect our software testability?

Share with your friends