CODEWITHSUNDEEP
javamethodsrefactoring
John Doe
John Doe

Reputation: 77

Comparing refactoring patterns in java

I have a simple method and I have applied two refactoring patters- Inline temp and Extract Method Followed by Replace Temp with Query. I would like to know which pattern is more preferable to use in our method. So, this is original method:

 public int mymethod(int a, int b) { 
    int t = a * b; 
    if (t > 100) { 
        return t * 0.65; 
    } else { 
        return t * 0.45; 
    } 
}

This is the method with Inline temp refactoring:

public int mymethod(int a, int b) {

        if (a*b > 100)
        { 
            return (a*b * 0.65);
        }
        else 
        { 
            return (a*b * 0.45);
        } 
    }

And finally Extract method followed by query refactoring:

public int mymethod(int a, int b) {
  if (t() > 100) {
    return t() * 0.65;
  }
  else {
    return t() * 0.45;
  }
}
int t() {
  return a * b;
}

Which one is better to use for our method, you think ? Personally, I think inline temp is better for this method, because Extract method has a new method and program is forced to query that extra method which hits the performance. Am I right ?

Upvotes: 0

Views: 122

Answers (1)

Rufi
Rufi

Reputation: 2645

There is no point of doing this type of refactoring.

Notice that in your original example you calculate the value for variable t once. And this is much better than doing this operation three times as you do in each of your "refactoring" examples. Moreover, notice that you do not pass a and b to your t method in second refactoring version plus return type in the original mymethod is wrong.

I would suggest to look into other things. Consider:

  1. Extracting constants.
  2. Giving meaningful names.
  3. Returning once.

If you apply those suggestions your method might change to something like this:

private static final int THRESHOLD = 100;
private static final double MAX_MEANINGFUL_NAME_CONSTANT = 0.65;
private static final double MIN_MEANINGFUL_NAME_CONSTANT = 0.45;

public double myMethod(int a, int b) {
    int multiplication = a * b;
    double result;
    if (multiplication > THRESHOLD) {
        result = multiplication * MAX_MEANINGFUL_NAME_CONSTANT;
    } else {
        result = multiplication * MIN_MEANINGFUL_NAME_CONSTANT;
    }
    return result;
}

From readability it looks better. And readability is quite important.

Upvotes: 2

Related Questions