HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Refactoring a method for estimation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
refactoringestimationformethod

Problem

I have to refactor this method: it seems to duplicate two code blocks with some slight variation. Any suggestions? It is a bit of a mess :)

public EstimatedAttitude estimate(int numStars, Double2d starsBrf, Double2d starsMeq, Long1d starsIdsForRange,
        double measerrRef, double probThresh) {

    Double2d starsBrfTry = new Double2d(9, 3);
    Double2d starsMeqTry = new Double2d(9, 3);

    Int1d idBad = new Int1d(2);
    int numBad = 0;

    //Estimate the attitude using q-method
    QMethodValues estimates = QMethod.compute(numStars, starsBrf, starsMeq, measerrRef);
    //double loss = estimates.getLoss();
    //Quaternion quat = estimates.getQuat();

    //Calculate probability of getting such a large value of TASTE at random
    double taste = estimates.getTaste();
    double gammaP = new GammaP(0.5 * (2 * numStars - 3)).calc(0.5 * taste);
    double prob = 1.0 - gammaP;

    //If no error and a bad fit
    if ((estimates.isValid()) && (prob = probThresh) && (tasteTry = probThresh) && (tasteTry < bestTaste)) {
                    //Make a note of the details
                    numBad = 2;
                    idBad.set(0, ((Long ) starsIdsForRange.get(badCand_1)).intValue());
                    idBad.set(1, ((Long ) starsIdsForRange.get(bad_cand_2)).intValue());
                    /*quat = estimatesTry.getQuat();
                    loss = estimatesTry.getLoss();
                    taste = tasteTry;*/
                    estimates = estimatesTry;
                    prob = probTry;
                }
            }

        }

    }

    //create a new qmethod value with correct fields
    return new EstimatedAttitude(estimates, prob, numBad, idBad);
}

Solution

Unit Tests

This is probably going to be a major rewrite, so the first thing I would do is write unit tests for it. That way, you can confirm that your new version actually will do the same thing as the original version.

Naming

The current names don't really tell a reader all that much:

  • starsBrf & starsMeq: what is a brf star? and what is a meq? if they are technical terms, describe them in a (JavaDoc) comment, if not, use better names.



  • loop isn't really a loop, it's a counter. but what does it count?



  • idBad & numBad: what is a bad? and what does the num do for it?



  • estimates: estimates of what?



  • what is a taste, and what is a try?



  • and so on.



You should also change the variables so that they comply with Java naming standards (camelCase instead of snake_case).

Remove commented out code

Remove all commented out code, as it makes the code harder to read. If you think that it might be needed in the future, use version control.

Comments

You need more comments in your code. At least you would need a JavaDoc for estimate. What does it estimate? What parameters does it except? How does it estimate?

Refactoring

Without working code, unit tests, and a general description of what the code actually does, it will be really hard for us to refactor this.

My general approach would be to identify similar code at a small scope, extract both versions of it to new methods (with appropriate arguments and return values), and then see if I can change the signature and workings of those methods to use the same method for both cases. If it does work (confirm correctness with unit tests), do the same thing for the next biggest scope.

During this process, I would also keep an eye out if creating objects might simplify some of the code.

Context

StackExchange Code Review Q#67785, answer score: 9

Revisions (0)

No revisions yet.