Reputation: 33
Okay, so I've got this class mostly figured out, but I can't seem to get gallonsSaved to work. What I want is for it to divide distanceTraveled by the mpg in order to get the gallons of gas you would have used if you'd used a car, but it just returns 0.0. I'm pretty sure my getters and setters are messed up, and not certain how to fix it. Why is it just returning a 0.0?
public class BikeCommute
{
private String route;
private double distanceTraveled;
private int timeRequired;
private String dateTraveled;
private String mode;
private double gallonsSaved;
final private double mpg = 20.8;
public BikeCommute(String mode, String dateTraveled, String route,
double distanceTraveled, int timeRequired)
{
this.mode = mode;
this.route = route;
this.distanceTraveled = distanceTraveled;
this.timeRequired = timeRequired;
this.dateTraveled = dateTraveled;
}
public double gallonsCalculated(double distanceTraveled, double mpg)
{
gallonsSaved = distanceTraveled/mpg;
return gallonsSaved;
}
public double getGallonsSaved()
{
return this.gallonsSaved;
}
public void setGallonsSaved(double gallonsSaved)
{
this.gallonsSaved = gallonsSaved;
}
public double getMpg() {
return mpg;
}
public String toString()
{
return mode + " " + route + ", " + distanceTraveled + " miles, " + timeRequired + " hrs, " +
dateTraveled + ". Gallons saved from switching from car: " + gallonsSaved;
}
}
Upvotes: 0
Views: 131
Reputation: 34628
Yes, you have a conceptual problem here.
You want the gallonsSaved to be a value calculated from other values. Usually, such values are not accessed through getters and setters, but through a calculation (class behavior).
There are usually two things you may do. The first is to have getters and setters for all the values that your behavior depends upon - distanceTraveled, in this case. You'd probably also have getters and setters for other fields, but they are not pertinent to the behavior we are currently discussing.
Then, you write a method that runs the behavior you need based on the values currently existing in your object (regardless of whether they were entered by constructor or by setter).
public double gallonsCalculated()
{
return distanceTraveled/mpg;
}
You will have no gallonsSaved
field because it depends on the other fields and therefore redundant. When you need it, you call that function.
The second approach, used especially when the calculation is called many times and its results are less likely to change, is to do the calculation once, store it in a private field, and return that field when you need the value.
In that case, you should perform the actual calculation whenever your setters are used:
public void setDistanceTravelled( double distanceTravelled ) {
this.distanceTravelled = distanceTravelled;
this.gallonsSaved = distanceTravelled/mpg;
}
public double gallonsCalculated() {
return this.gallonsSaved;
}
It's done like this so that the internal value of gallonsSaved
always matches the current distanceTravelled
.
If there is no setter for distanceTravelled
and it's only set from the constructor, you can do the calculation in the constructor itself and it will then never change. You can even declare gallonsSaved
final if you do that.
The reason you keep getting 0 is that you seem to have picked the second option - keeping a hidden variable for the result of a calculation - but you never performed the calculation and stored it in it.
Upvotes: 1
Reputation: 19613
gallonsCalculated
should probably used this.distanceTraveled
, instead of taking distanceTraveled
as a parameter. Unless you're trying to set this.distanceTraveled
there, in which case you're forgetting to assign to it.
Upvotes: 0