SmallCar
...
// returns the number of whole Litres of fuel consumed during the journey
public int drive(int distance) {
int fuelUsed = 0;
if (distance < 0) {
throw new IllegalArgumentException("Distance cannot be less than zero");
}
if (super.isRented() && (super.getCurrentFuel() > 0)) {
fuelUsed = (distance / 25);
super.setCurrentFuel(super.getCurrentFuel() - fuelUsed);
return fuelUsed;
}
return fuelUsed;
}
Multiple things that can be better here:
First, guard clauses like
if (distance < 0) {
throw new IllegalArgumentException("Distance cannot be less than zero");
}
They go at the top of the function where possible. Before any variable declarations. No need to reserve memory for something that you're not going to use. Also, it allows you to first perform checks, then have ALL the code below assume sane values.
if (super.isRented() &&
Apparently, non-rented cars cannot drive. But yours don't notify the caller. They'll happily report the trip as having taken 0 liters of fuel. I want to buy one of those cars. Not having to pay for fuel would be great.
fuelUsed = (distance / 25);
Every car has a different milage, and yours is infinite kilometers per liter, provided the car has a liter of fuel to start with, and that the trip doesn't go over 25 kilometers. I've got about a 20km commute, so that's great. I REALLY want one of these cars. Joking aside, integer division is a real problem with this sort of thing and that's why distances and amounts of an item of which there can only be many (such as water or fuel, you do not have 1 fuel, you have 1 liter or 0.5 or 0.05 liter of fuel), tend to be floating point numbers. Real numbers. You'll probably have to use double
for them.
return fuelUsed;
}
return fuelUsed;
This is redundant.
public boolean isFull() {
boolean bool = super.getIsFull();
return bool;
}
Why do you have this method? People can just call getIsFull
(or isTankFull
or maybe hasFullTank
, see points on naming earlier).