CODEWITHSUNDEEP
javaif-statementreturnbooleansetter
Christian
Christian

Reputation: 23

Setter - Checking the value

I am supposed to check the value and if it's not the value, I do not update the value. And if the input is valid then I return it.

Minimal reproducible example:

public class Student {

    private int studentId;
    private String name;
    private double grade;
    private double multiplier;


    public double getMultiplier() {
        return multiplier;
    }

    /**
     * The setter for the multiplier must check that the value is either 1.08 *
     * 1.06 or 1.08 or 1.06
     * 
     * If not, then do not update the value
     * 
     * @param multiplier
     * @return if the input was valid
     */
     public boolean setMultiplier(double multiplier) {

         if( multiplier == 1.08 * 1.06 || multiplier == 1.08 || multiplier == 1.06 ) { }
         return multiplier;
    }
    ...
}

Upvotes: 2

Views: 579

Answers (4)

deHaar
deHaar

Reputation: 18568

Possibly a use case for constants:

private static final double ONE_DOT_ZERO_SIX = 1.06f;
private static final double ONE_DOT_ZERO_EIGHT = 1.08f;

/**
 * Just to know if it is valid
 */
public boolean setMultiplier(double multiplier) {
    return (multiplier == ONE_DOT_ZERO_SIX || multiplier == ONE_DOT_ZERO_EIGHT
            || multiplier == (ONE_DOT_ZERO_SIX * ONE_DOT_ZERO_EIGHT));
}

/**
 * Return the multiplier if it is valid, otherwise 1
 */
public double setMultiplier(double multiplier) {
    if (multiplier == ONE_DOT_ZERO_SIX || multiplier == ONE_DOT_ZERO_EIGHT
            || multiplier == (ONE_DOT_ZERO_SIX * ONE_DOT_ZERO_EIGHT)) {
        return multiplier;
    } else {
        // return any invalid multiplier, this one doesn't change values at least
        return 1;   
    }
}

/**
 * Set the correct multiplier or a substitute
 */
public void setMultiplier(double multiplier) {
    if (multiplier == ONE_DOT_ZERO_SIX || multiplier == ONE_DOT_ZERO_EIGHT
            || multiplier == (ONE_DOT_ZERO_SIX * ONE_DOT_ZERO_EIGHT)) {
        this.multiplier = multiplier;
    } else {
        // return any invalid multiplier, this one doesn't change values at least
        this.multiplier = 1;
    }
}

Upvotes: 0

pb11pb
pb11pb

Reputation: 148

public void setMultiplier(double multiplier) { // method head

    if (multiplier == 1.08 * 1.06 || multiplier == 1.08 || multiplier == 1.06) {
        this.multiplier = multiplier; // here the field variable is overwritten
    }

    throw new IllegalArgumentException("exception message");
}

You forgot to write the field variable of your class.

A setter should override a field variable. A setter should override a field variable. He does not give anything back therefore void instead of boolean in the method head. If the parameters are wrong, throw an exception (as meaningful as possible).

Tip: I would not distribute my constants like 1.06 or 1.08 anywhere in the code. You could define it as follows:


public class student {

    private static final double MEANINGFUL_NAME_0 = 1.06;
    private static final double MEANINGFUL_NAME_1 = 1.08;

    // other code below
}

Advantage: If necessary, you have to change the constant only in one place. The likelihood that you will miss out and write 1.07 instead of 1.06 is lower.

Upvotes: 2

THess
THess

Reputation: 1007

I think you just forgot to actually set the new value.

Therefore it should work if you do it like this:

public void setMultiplier(double multiplier) {


    if( multiplier == 1.08 * 1.06 || multiplier == 1.08 || multiplier == 1.06 ) {
        this.multiplier = multiplier;
    }
}

Also, setters are usually void instead of boolean and have no return value.

Upvotes: 0

azro
azro

Reputation: 54148

A setter should update a value, and doesn't return a value (getter's role) so it's return type should be void and not boolean

public void setMultiplier(double multiplier) {
    if( multiplier == 1.08 * 1.06 || multiplier == 1.08 || multiplier == 1.06 ) {
        this.multiplier = multiplier; 
    }
}

Upvotes: 0

Related Questions