Duplicating code for throwing exceptions

Question

lets say i have this code:

public class Car {
    private int fuel;

    public Car(int fuel) {
        if (fuel < 0) {
            throw new IllegalArgumentException("Can't be negative");
        }
        this.fuel = fuel;
    }

    public void setFuel(int fuel) {
        if (fuel < 0) {
            throw new IllegalArgumentException("Can't be negative");
        }
        this.fuel = fuel;
   }

My question is, can I somehow avoid duplicating code in constructor and setter?

By calling the setter from the constructor? (Note that you might want to make the setter final to avoid [calling an overridable method from the constructor](http://stackoverflow.com/questions/3404301/whats-wrong-with-overridable-method-calls-in-constructors).) — aioobe, Nov 15 '15 at 10:26

score 4 · Answer 1 · answered Nov 15 '15 at 10:26

4

Why don't you change the constructor to call the setter:

public Car(int fuel) {
    setFuel(fuel);
}

answered Nov 15 '15 at 10:26

krock

28,904
13
79
85

1

I also thought about that, but then read this: https://stackoverflow.com/questions/4893558/calling-setters-from-a-constructor – kkralj Nov 15 '15 at 10:29
1

In Jon Skeets accepted answer he prefers to keep classes immutable. So in his case there is no setter and all the logic is contained in the constructor. If you want setters and mutable objects though, it is preferable to reuse code whenever you can. – krock Nov 15 '15 at 10:35
1

@krock: That's a *follow-on* aspect to the answer. What Jon primarily says is: *"I would set the variable directly in most cases. Methods usually expect that the instance is fully-formed by the time they're called."* (And absolutely, making objects immutable when possible tends to be a useful design choice. It's frequently not possible.) – T.J. Crowder Nov 15 '15 at 10:38

score 1 · Answer 2 · answered Nov 15 '15 at 10:26

Call the setter from the constructor directly:

public class Car {
    private int fuel;

    public Car(int fuel) {
        setFuel(fuel);
    }

    public void setFuel(int fuel) {
        if (fuel < 0) {
            throw new IllegalArgumentException("Can't be negative");
        }
        this.fuel = fuel;
   }

score 1 · Answer 3 · answered Nov 15 '15 at 10:26

1

Yes. Invoke setFuel (fuel) from within your constructor.

answered Nov 15 '15 at 10:26

Jan

13,738
3
30
55

score 1 · Accepted Answer · edited May 23 '17 at 12:23

As with most duplicated code, you can factor it out into a method.

public class Car {
    private int fuel;

    public Car(int fuel) {
        checkFuel(fuel);
        this.fuel = fuel;
    }

    public void setFuel(int fuel) {
        checkFuel(fuel);
        this.fuel = fuel;
    }

    private static void checkFuel(int fuel) {
        if (fuel < 0) {
            throw new IllegalArgumentException("'fuel' argument can't be negative");
        }
    }

Why not call setFuel from the constructor? Because calling methods of your instance from within the constructor before you've completely initialized can be a source of errors. Consider: A subclass overrides setFuel and adds side-effects. More about that in this question and its answers.

Here's another way, also encapsulating it in a method:

public class Car {
    private int fuel;

    public Car(int fuel) {
        this.privateSetFuel(fuel);
    }

    public void setFuel(int fuel) {
        this.privateSetFuel(fuel);
    }

    private void privateSetFuel(int fuel) {
        if (fuel < 0) {
            throw new IllegalArgumentException("'fuel' argument can't be negative");
        }
        this.fuel = fuel;
    }

Side note: Notice I added the name of the argument to the exception message.

Duplicating code for throwing exceptions

4 Answers4