Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

is this too much refactoring?

I try to refactor a code so that it will use separate methods to do some calculations. Just to make it clear.

What I want to know is, is it a good practice or a bad one to write a separate method to find out a simple thing like a number is odd or even ?

The original code is ,

    int n = 11;
    if (n % 2 == 0) {
        System.out.println("Not selected");
    } else {
        boolean isPrime = true;
        if (n == 0 || n == 1) {
            isPrime = false;
        } else {
            int i = 2;
            double a = Math.sqrt(Math.abs(n));
            while (i <= a) {
                if (n % i == 0) {
                    isPrime = false;
                }
                ++i;
            }
        } 
        if(isPrime){
            System.out.println("Prime it is");
        }
    }

The refactored code is,

    int n = 11;
    if (isEven(n)) {
        System.out.println("Not selected");
    } else { 
        if (isPrime(n)) {
            System.out.println("Prime it is");
        }
    }

public static boolean isEven(int n) {
    return n % 2 == 0 ? true : false;
}

public static boolean isPrime(int n){
    if(n==0 || n==1)return false;        
    int i=2;
    double a = Math.sqrt(Math.abs(n));
    while(i<=a){
        if(n%i==0){
            return false;
        }            
        ++i;
    }
    return true;
}
like image 286
prime Avatar asked Jul 26 '15 07:07

prime


2 Answers

It's generally considered good practice to break code down into separate methods for things like readability, length, or cyclomatic complexity, especially if you are not changing how the code works.

Boolean expressions, like what you have extracted, are often good choices for a quick extract function refactor. It allows a reader of the code base to know why a boolean expression is important or what it does by being able to read a descriptive function name versus complex domain related boolean math which they may not care to know the intricate details of.

A good book about commonly considered best practices in Java for code organization is a book called Clean Code. It's a pretty easy and enjoyable read, I would suggest it.

like image 156
NESPowerGlove Avatar answered Oct 17 '22 12:10

NESPowerGlove


This is referred to as functional decomposition, and (in my opinion) is always good practice.

Not only is it easier to read, you can find problems in that code easier, since you can now easily test each section individually.

For example, you can now make sure isEven actually returns an even number. If a problem arises later on, you know that method is not the issue. This is called unit testing.


I suggest switching

if(isEven(n)) {

} else {
    if(isPrime(n)) {

    }
}

to

if(isEven(n)) {

} else if(isPrime(n)) {

}

Since it gives the same functionality, yet saves you a line.

Although, if you were counting 0 as being even, you wouldn't even need isPrime; if it's not even, it has to be prime. You wouldn't need to perform all the processing in isPrime:

if(isEven(n)) {
    System.out.println("Even");
} else {
    System.out.println("Odd");
}

To make it a bit cleaner:

String result = isEven(n) ? "Even" : "Odd";
System.out.println(result);
like image 30
Vince Avatar answered Oct 17 '22 12:10

Vince