Need refactoring opinions

[mevtho]

Hi,
I have to refactor the method kingRange under, the goal being code readability :

/*
For information, here are the class fields needed here :
private String setUp;
public final static Collection validPieces =
    Arrays.asList(new String[] { "R", "N", "B", "K", "Q" });
public static final int LEFT = 1;
public static final int RIGHT = 2;

*/

public Range kingRange() {
    Range result;
    
    if ( (setUp.indexOf("R",setUp.indexOf("R")+1) + 1) > 
             (setUp.indexOf("R") + 1) ) {                
        result = new Range((setUp.indexOf("R") + 1) + 1, (setUp.indexOf("R",setUp.indexOf("R")+1) + 1) - 1);
    }
    else {
        int firstEmptyPositionReadingLeftToRight = 0;
        boolean find = false;
        for (int i = 1; i < 8; i++) {
            if (!find && !validPieces.contains(setUp.substring(i-1,i))){
                firstEmptyPositionReadingLeftToRight = i;
                find = true;
            }
        }
        
        int firstEmptyPositionReadingRightToLeft = 0;
        find = false;
        for (int i = 8; i > 1; i--) {
            if (!find && !validPieces.contains(setUp.substring(i-1,i))){
                firstEmptyPositionReadingRightToLeft = i;
                find = true;
            }
        }

        int left = 0;
        int right = 0;
        if ((setUp.indexOf("R") + 1) ==
               firstEmptyPositionReadingLeftToRight - 1) {
            left = firstEmptyPositionReadingLeftToRight;
        } else {
            left = firstEmptyPositionReadingLeftToRight + 1;
        }
        if ((setUp.indexOf("R") + 1) == 
               firstEmptyPositionReadingRightToLeft + 1) {
            right = firstEmptyPositionReadingRightToLeft;
        } else {
            right = firstEmptyPositionReadingRightToLeft - 1;
        }
        result = new Range(left, right);
     
    }
    return result;
}

Here is where I am so far :

public Range kingRange() {        
    if ( (setUp.indexOf("R",setUp.indexOf("R")+1) + 1) > 
         (setUp.indexOf("R") + 1)) {
        return new Range((setUp.indexOf("R") + 1) + 1, (setUp.indexOf("R",setUp.indexOf("R")+1) + 1) - 1);
    }
    return new Range(rangeLeft(), rangeRight());                
}

private int rangeRight() {
    int firstEmptyPositionReadingRightToLeft = 
        firstEmptyPositionReadingRightToLeft();
    
    if ((setUp.indexOf("R") + 1) == 
            firstEmptyPositionReadingRightToLeft + 1) {
        return  firstEmptyPositionReadingRightToLeft;
    } 
    return firstEmptyPositionReadingRightToLeft - 1;
}

private int rangeLeft() {
    int firstEmptyPositionReadingLeftToRight =
        firstEmptyPositionReadingLeftToRight();
    
    if ((setUp.indexOf("R") + 1) ==
            firstEmptyPositionReadingLeftToRight - 1) {
        return firstEmptyPositionReadingLeftToRight;
    }
    return firstEmptyPositionReadingLeftToRight + 1;
}

private int firstEmptyPositionReadingRightToLeft() {
    for (int i = 8; i > 1; i--) {
        if (!validPieces.contains(setUp.substring(i - 1, i))) {
            return i;
        }
    }
    return 0;
}

private int firstEmptyPositionReadingLeftToRight() {
    for (int i = 1; i < 8; i++) {
        if (!validPieces.contains(setUp.substring(i - 1, i))) {
            return i;
        }
    }
    return 0;
}

My question about it is : do you think that some improvement can still be done using Fowler refactorings (see http://www.refactoring.com, in particular removing duplicate code, merging methods, ...) without sacrificing the code readability and understandability ?

I don't want a solution, just your opinion, and if you want some clues on how to improve, I want to find things by myself.

Thanks.

[ithelp]

I think you have done enough refactoring, also put some comments in the code so that it becomes more understandable.

[mevtho]

Thanks for your answer, I think that too since to go further it would require to do deep changes, to the detriment of its understandability.

Any other views on it ?

[Ezzaral]

Notice that the only difference in those pairs of methods is the direction of advance, +1 or -1. If you see that much duplication between methods, chances are you can make them one method. Consider how it might work if you defined

final static int LEFT = -1;
final static int RIGHT = 1;

and passed the direction to search as a parameter to a single method.

[jwenting]

introduce some Enums to replace all those literals.

[mevtho]

Thank you for your answers, I've taken all you told me into account, and have been able to pursue my refactoring.

Thanks