validity checks

As you say, the problem statement includes this:

Check within the constructor the given values for being valid.

Here's a plane that your code does not reject as invalid:

new Plane(-5, 12, 1000, 800, 500, 2500, 25000, 8000); 

Not only can maxWeight (and the other parameters) invalidly be negative, it can also be exceeded by the the loadWeight.

units

Planes travel large distances, possibly reaching countries far far away, where people might use the imperial system (or even other, more arcane ones). You have an implicit contract of what units the values should be given in. That contract is not clearly communicated. Are the weights in metric tonnes? The fuel in gallons?

fun fact: The Concorde was constructed by British and French companies, one using imperial system and the other metric. Imperial units were used for the sections designed by the British team and metric units for those designed by the French team.

And even within the metric system for example, do you use kg, t, g, … for weight? You have to clearly document what units you are expecting.

Ideally, you'd use some built-in mechanism to work with units. That would even solve the imperial/metric conversion problem. Basically speaking, the dimension of a variable is specified and then the conversion of units for that dimension. If the dimension is temperature, you can shove any value into it that's a temperature, be it °C, °F, °Ré, °Rø or whathaveyou. Sadly, the proposal linked to above was withdrawn, because nobody cared to do it. I guess there are other libraries in place that do this. It's out of scope for a beginner program and nobody expects you to use a system like this. This paragraph is just FYI.

use min()/max() for clamping

If you want to keep a value within some limits, you can do this:

this.kerosinStorage = kerosinStorage < this.maxFuel ? kerosinStorage : this.maxFuel; 

but you can also do it this way

this.kerosinStorage = Math.min(kerosinStorage, this.maxFuel); 

which is a bit more self documenting if you got used to it, because it clearly says: take the smaller value. Parsing the ternary operator is a bit harder to read.

This might be personal preference. There's also a (blurry) line where simplifying code becomes obfuscation to cryptic one-liners. Don't be tempted to write everything in as little code as possible. You have to draw that line for yourself.

the validity checks strike back

Even if you have some validity checks, you have to come up with a contract on how they are enforced. What should happen if some invalid value is passed to your class? Maybe you want to throw newIllegalArgumentException("Negative weights aren't possible.") or maybe you want to limit the values to a certain range of valid values and move on. There is no clear right or wrong here. It's a matter of how you design your class. What's important is that you communicate whatever you do clearly. Even if you don't perform any validity checks, you should explain this to possible users of your class: "Hey there, if you pass nonsense to this class, it won't check for that and you have to live whatever garbage it produces as a result." This could be a valid choice in some situations. (In yours it's obviously not, because you were asked to do them)

naming

This is a subjective one. When I read kerosinStorage, I associate with that the size of the tank or container that the kerosene is stored in. In your code it is the actual amount of kerosene left in the plane. I suggest to change that name to just kerosene.

setters should set, not add

The setter for kerosinStorage adds to the current value and doesn't set it

public void setKerosinStorage(double kerosinStorage) { this.kerosinStorage = this.kerosinStorage + kerosinStorage > maxFuel ? maxFuel : this.kerosinStorage + kerosinStorage; } 

Either make this method a setter or rename it to addKerosene or something like that.

the return of the validity checks

Admittedly, this is more about code duplication, but I could not resist completing the trilogy.

Check within the constructor the given values for being valid.

That's a bit misleading, because if you have setters for properties, you should perform the validity checks in the setters, not in the constructor. As you have it right now, you have duplicated code to limit the value of kerosinStorage. You do it once in the constructor:

 this.kerosinStorage = kerosinStorage < this.maxFuel ? kerosinStorage : this.maxFuel; 

and then you do it again in "addKerosene":

 this.kerosinStorage = this.kerosinStorage + kerosinStorage > maxFuel ? maxFuel : this.kerosinStorage + kerosinStorage; 

Here's a reduced version of your class, that does things differently:

package plane; public class Plane { private double maxFuel; private double kerosene; public Plane(double maxFuel, kerosene) { this.maxFuel = maxFuel; setKerosene(kerosene); } public void setKerosene(double amount) { kerosene = Math.min(maxFuel, amount); } public void addKerosene(double amount) { setKerosene(kerosene + amount); } } 

In your current code, you have some validity checks in the setter methods fillUp() and load(), but they are not called in the constructor.

To wrap up the whole validity check thing once and for all:

• perform validity checks in the setter methods
• call the setter methods from the constructor to ensure they are performed and to prevent code duplication
• come up with a consistent way to deal with invalid values and communicate it clearly

Don't make your Class members private if you offer public getters and setters without any functional logic. Just make your members public. This saves you 70 (i.e. 7*2*5) lines if I counted correctly. It makes your class more readable, the access of the variables less complicated and saves Bytecode.