How to get rid of multiple if statements in java?-CodePudding

Is there a better way to write this constructor which has multiple if statements and multiple arguments? I'm a noob to programming so any leads would be helpful.

public Latency(final double full, final double cpuOne, final double cpuTwo, final double cpuThree, final double cpuFour) {
    if (full > 10.0 || (full <= 0.0)) {
        throw new IllegalArgumentException("Must check the values");
    }
    this.full = full;
    if (cpuOne == 0 && cpuTwo == 0 && cpuThree == 0 && cpuFour == 0) {
        throw new IllegalArgumentException("not all can be zero");
    } else {
        if (cpuOne == 0.5) {
            this.cpuOne = full;
        } else {
            this.cpuOne = cpuOne;
        }
        if (cpuTwo == 0.5) {
            this.cpuTwo = full;
        } else {
            this.cpuTwo = cpuTwo;
        }
        if (cpuThree == 0.5) {
            this.cpuThree = full;
        } else {
            this.cpuThree = cpuThree;
        }
        if (cpuFour == 0.5) {
            this.cpuFour = full;
        } else {
            this.cpuFour = cpuFour;
        }
    }
}

I think this code doesn't need much of context as it is pretty straight forward.

I found out that we can't use switch statements for type double. How to optimize this?

CodePudding user response：

There are a number of possible ways of refactoring the code that you've written, and there are pros and cons of each one. Here are some ideas.

Idea One - use the conditional operator

You could replace the else block with code that looks like this. This is just effectively a shorter way of writing each of the inner if/else blocks. Many people find this kind of form more readable than a bunch of verbose if/else blocks, but it takes some time to get used to it.

this.cpuOne = cpuOne == 0.5 ? full : cpuOne;
this.cpuTwo = cpuTwo == 0.5 ? full : cpuTwo;
this.cpuThree = cpuThree == 0.5 ? full : cpuThree;
this.cpuFour = cpuFour == 0.5 ? full : cpuFour;

Idea Two - move common functionality to its own method

You could have a method something like this

private static double changeHalfToFull(double value, double full) {
   if (value == 0.5) {
       return full;
   } else {
       return value;
   }
}

then call it within your constructor, something like this.

this.cpuOne = changeHalfToFull(cpuOne);
this.cpuTwo = changeHalfToFull(cpuTwo);
this.cpuThree = changeHalfToFull(cpuThree);
this.cpuFour = changeHalfToFull(cpuFour);

This has the advantage that the key logic is expressed only once, so it's less error prone than repeating code over and over.

Idea Three - use arrays

You could use an array of four elements in the field that stores these values. You could also use an array for the constructor parameter. This has a huge advantage - it indicates that the four CPU values are somehow all "the same". In other words, there's nothing special about cpuOne compared to cpuTwo, for example. That kind of messaging within your code has real value to someone trying to understand this.

public Latency(final double full, final double[] cpuValues) {

// validation conditions go here ...

    this.cpuValues = new double[4];
    for (int index = 0; index <= 3; index  ) {
        if (cpuValues[index] == 0.5) {
             this.cpuValues[index] = full;
        } else {
             this.cpuValues[index] = cpuValues[index];
        }
    }
}

Or a combination

You could use some combination of all these ideas. For example, you might have something like this, which combines all three of the above ideas.

public Latency(final double full, final double[] cpuValues) {

// validation conditions go here ...

    this.cpuValues = new double[4];
    for (int index = 0; index <= 3; index  ) {
        this.cpuValues[index] = changeHalfToFull(cpuValues[index]);
    }
}


private static double changeHalfToFull(double value, double full) {
    return value == 0.5 ? full : value;
}

There are obviously other possibilities. There is no single correct answer to this question. You need to choose what you're comfortable with, and what makes sense in the larger context of your project.

CodePudding user response：

DRY - Don't Repeat Yourself

Each if is essentially the same. Put it in a separate method and call the method once for each cpu* variable.

public class Latency {
    private double full;
    private double cpuOne;
    private double cpuTwo;
    private double cpuThree;
    private double cpuFour;

    public Latency(final double full,
                   final double cpuOne,
                   final double cpuTwo,
                   final double cpuThree,
                   final double cpuFour) {
        if (full > 10.0 || (full <= 0.0)) {
            throw new IllegalArgumentException("Must check the values");
        }
        this.full = full;
        if (cpuOne == 0 && cpuTwo == 0 && cpuThree == 0 && cpuFour == 0) {
            throw new IllegalArgumentException("not all can be zero");
        }
        else {
            this.cpuOne = initCpu(cpuOne);
            this.cpuTwo = initCpu(cpuTwo);
            this.cpuThree = initCpu(cpuThree);
            this.cpuFour = initCpu(cpuFour);
        }
    }

    private double initCpu(double cpu) {
        return cpu == 0.5 ? full : cpu;
    }

    public static void main(String[] arg) {
        new Latency(9.99, 8.0, 7.0, 6.0, 0.5);
    }
}