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.) — aioobe
– aioobe, Commented Nov 15, 2015 at 10:26

krock · Accepted Answer · 2015-11-15 10:26:15Z

4

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

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

answered Nov 15, 2015 at 10:26

krock

29.7k13 gold badges81 silver badges86 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

kkralj Over a year ago

I also thought about that, but then read this: stackoverflow.com/questions/4893558/…

krock Over a year ago

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.

T.J. Crowder Over a year ago

@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.)

Community · Accepted Answer · 2017-05-23 12:23:37Z

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.

Mohammed Aouf Zouag · Accepted Answer · 2015-11-15 10:26:03Z

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; }

Jan · Accepted Answer · 2015-11-15 10:26:16Z

1

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

answered Nov 15, 2015 at 10:26

Jan

13.9k3 gold badges35 silver badges58 bronze badges

Collectives™ on Stack Overflow

Duplicating code for throwing exceptions

4 Answers 4

3 Comments

Comments

Comments

Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

3 Comments

Comments

Comments

Comments

Linked

Related