I have been in the software development industry since 2010, working on enterprise product development using ADF. I am usually keen on learning about software design and emerging technologies. You can find me hanging around in the JavaRanch Forums where I am one of the moderators. Apart from Java, I am fascinated by the ease of use and simplicity of Ruby and Rails. Mohamed is a DZone MVB and is not an employee of DZone and has posted 53 posts at DZone. You can read more from them at their website. View Full User Profile

Refactor Out Code Smell With JDeodrant

03.27.2012
| 6774 views |
  • submit to reddit

I got to know about JDeodorant- a tool for identifying bad smells in code and helping it to refactor. I got curious and downloaded its Eclipse plugin, I then picked the first bad smell code which Martin Fowler explains in his book: “Refactoring: Improving the design of existing code”. I tried my hand at refactoring a long method by Extract Method refactor move. Here’s the long method:

public String statement(){
  double totalAmount = 0;
  int frequentRenterPoints = 0;
  String result = "Rental record for "+ getName() + "\n";
  for(Rental each : _rentals){
    double thisAmount = 0;
    switch (each.getMovie().getPriceCode()) {
      case Movie.REGULAR:
        thisAmount += 2;
        if ( each.getDaysRented() > 2)
          thisAmount += (each.getDaysRented() - 2 ) * 1.5;
        break;
      case Movie.NEW_RELEASE:
        thisAmount += each.getDaysRented() *3;
        break;
      case Movie.CHILDREN:
        thisAmount += 1.5;
        if (each.getDaysRented() > 3 )
          thisAmount += (each.getDaysRented() - 3)* 1.5;
        break;
    }
    frequentRenterPoints++;
    if ( (each.getMovie().getPriceCode() == Movie.NEW_RELEASE)
          && each.getDaysRented() > 1 )
      frequentRenterPoints ++;
    result += "\t" + each.getMovie().getTitle() + "\t" +
        String.valueOf(thisAmount) + "\n";
    totalAmount += thisAmount;
  }
  result += "Amount owed is "+ String.valueOf(totalAmount)+"\n";
  result += "You earned " + String.valueOf(frequentRenterPoints) +
      " frequent renter points";
  return result;
}

So I use the JDeodorant plugin and run the Long Method: Bad Smell, it correctly identifies the thisAmount calculation as a long method, as shown in the image:

Now I try to incorporate the suggested refactor and it leads to the code where the thisAmount calculation is moved into a method getAmount:

private void getAmount() {
  for (Rental each : _rentals) {
    double thisAmount = 0;
    switch (each.getMovie().getPriceCode()) {
      case Movie.REGULR:
        thisAmount += 2;
        if (each.getDaysRented() > 2) {
          thisAmount += (each.getDaysRented() - 2) * 1.5;
        }
        break;
      case Movie.NEW_RELEASE:
        thisAmount += each.getDaysRented() * 3;
        break;
      case Movie.CHILDREN:
        thisAmount += 1.5;
        if (each.getDaysRented() > 3) {
          thisAmount += (each.getDaysRented() - 3) * 1.5;
        }
        break;
    }
  }
}

and the original code is altered as:

//older code
thisAmount = getAmount(); //new line added after refactor
for(Rental each : _rentals){
  frequentRenterPoints++;
//code here moved to getAmount method
//older code
  if ( (each.getMovie().getPriceCode() == Movie.NEW_RELEASE)
      && each.getDaysRented() > 1 )
    frequentRenterPoints ++;

The identification of the parts which could be refactor was correct but the refactor done wasn’t as expected. Ideally the variables which the block of code was referring to should have moved to the method param list and the code identified for refactor copied as is. And also as this refactor was around the use of a certain variable “thisAmount”, the extracted method should have returned the calculated value. So I might have to explore the way the refactoring is done.

But to start with:
- It helps in identifying bad smells – I tried with the one which was obvious, not sure about the not so obvious ones.
- One can consider the refactoring advice but not always will the judgement of a tool be better than human experience. So take it with a pinch of salt.

I would like to explore this as and when I get time, but I would also have to read the ideas behind the bad smell identification and refactoring suggestions before I can move along with the tool.

Just in case someone has already tried using this tool, please feel free to drop in the feedback.

And an interesting name for the project: JDeodorant- to drive away the bad smell :)

 

 

 

Published at DZone with permission of Mohamed Sanaulla, author and DZone MVB. (source)

(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)

Comments

Mikael Couzic replied on Wed, 2012/03/28 - 4:01am

I'm sorry, but I don't really see the point. Why do you need an Eclipse plugin to do something that obvious ? Show us an example where a bad smell could go undetected, and where the needed refactoring is a bit more sophisticated !

Somehow, I doubt such examples exist. However, I think this tool can be pretty useful in an educational context. It can help novice developers learn to write cleaner code.

Senthil Balakrishnan replied on Wed, 2012/03/28 - 8:22am

+1 Mikael Couzic, I am a bit lost too. A better example would have helped to co-relate the benefit of using is tool.

Mark Unknown replied on Wed, 2012/03/28 - 1:15pm

Sadly, for many, many developers, it is NOT obvious.  Things like 'rental' instead of 'each'.  That is why tooling like this is necessary.  I am not sure how it is mathmatically possible, but the majority of developers are below average. ;)

Mohamed Sanaulla replied on Thu, 2012/03/29 - 3:45am in response to: Senthil Balakrishnan

There's lot of research involved in how JDeodrant identifies the code smells. They have research papers linked from their project site. I was going through 1 (about Extract method) and its more of what we do manually which is being automated. I havent got time to read other papers on this. 

I am not sure how much of industry applicable it can be, I was curious as to how they would do the refactoring ( also driven by the fact that I was reading Martin Fowlers Refactoring ;) ) so thought of trying out the tool. As pointed out a better example would have been much useful and also the fact that other options for refactoring were not tried.  

Mohamed Sanaulla replied on Thu, 2012/03/29 - 3:49am in response to: Mikael Couzic

Yes this was a very obvious example, as pointed in my other comment there has been lot of research put in this and there by a tool was developed (an eclipse plugin) as an implementation of the research. 

 The research done on this is a pretty good read 

Mikael Couzic replied on Wed, 2012/04/04 - 5:29am in response to: Mark Unknown

I don't think that the developers you refer to are all bad by nature, they're just uneducated. A tool like JDeodorant can help them learn, but on the long term I doubt it would still be useful.

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.