Avoid if/else in constructor - NavigableMap?

Question

I have the following code, based on the input to the constructor I need to initialise some values which are wrapped in a simple POJO.. These values are constant as you can see.

However I don't like the if/else construct and had read about NavigableMap as an alternative for holding range values.. Any thoughts on how to improve/clean the below or indeed use this construct?

Thanks

private Calibrated calibratedValues;

public CalibrationProperties(long odNumber) {
    setCalibratedValues(odNumber);
}

private void setCalibratedValues(long odNumber) {
    if (odNumber < 762) {
        this.calibratedValues = new Calibrated(K0, H0, K0_INV, H0_INV, F0);
    } else if (odNumber < 866) {
        this.calibratedValues = new Calibrated(K1, H1, K0_INV, H0_INV, F1);
    } else if (odNumber < 1011) {
        this.calibratedValues = new Calibrated(K2, H2, K2_INV, H2_INV, F2);
    } else {
        this.calibratedValues = new Calibrated(K3, H3, K3_INV, H3_INV, F3);
    }

    //Two exceptions
    if (odNumber == 858){
        this.calibratedValues = new Calibrated(K2, H2, K2_INV, H2_INV, F2);
    }
    if (odNumber == 1005){
        this.calibratedValues = new Calibrated(K3, H3, K3_INV, H3_INV, F3);
    }
}

public Calibrated getCalibratedValues() {
    return calibratedValues;
}

/** Convenience class used to hold calibrated values */
static class Calibrated {
    private double[] k;
    private double[] h;
    private double[] kinv;
    private double[] hinv;
    private double f;

    Calibrated(double[] k, double[] h, double[] kinv, double[] hinv, double f) {
        this.k = k;
        this.h = h;
        this.kinv = kinv;
        this.hinv = hinv;
        this.f = f;
    }

    public double[] getK() {
        return k;
    }

    public double[] getH() {
        return h;
    }
    ...

This question appears to be off-topic because it is about code review. Please try [codereview.stackexchange.com](http://codereview.stackexchange.com/) instead. — Sergey Kalinichenko, Oct 08 '14 at 13:08
You want the values to be assigned with some criteria and you don't want if..else condition???? or you don't want any conditional structure?? I don't exactly understand your question. But if you just don't want use if/else than you can use Switch case. only if you want some condition based structure... — emphywork, Oct 08 '14 at 13:11
There is a nice discussion here about if..else statements [here](http://stackoverflow.com/questions/16849656/design-pattern-to-implement-business-rules-with-hundreds-of-if-else-in-java), basically one of the best approaches seems to be a Strategy pattern.. — Serhiy, Oct 08 '14 at 13:25

score 0 · Answer 1 · answered Oct 08 '14 at 13:11

You are not using if/else in constructors here, but you have created something like factory method.

use more meaningful names, if you will read this code after a year you will not know what they mean - I mean constructor parameters and class properties
your constructor has too large argument list. You could create inside of your class some factory methods

Example:

public static Calibrated createSmallCaribration(){
   return new Calibrated(K0, H0, K0_INV, H0_INV, F0);
}

If you will do that you can even make your contructor package visible. And you will know what each constructor configuration is ment to be,

Is is more code review and discussion about clean code principals in practise:) But oyr could start with few changes mentioned above:) — Beri, Oct 08 '14 at 13:14

Avoid if/else in constructor - NavigableMap?

1 Answers1