Calculating min and max with if-statements within a for-loop

Question

This code section is from the main method of my Annual Fuel Use class. The program projects my annual fuel usage based on at least three fill ups of my car. Here, I am calculating the max and min for distance (which is miles traveled), MPG, and price per gallon. I'd like a general review.

//initialization of array of objects
AnnualFuelUse[] fillUps = {new AnnualFuelUse (1, 1, 6500, 6800, 9.70, 3.11),
                          new AnnualFuelUse (2, 10, 6800, 7052, 8.10, 3.08),
                          new AnnualFuelUse (3, 20, 7052, 7349, 9.20, 3.15)};

//calculate Min and Max for distance, MPG, and price per gallon
double minDist = 0, maxDist = 0;
double minMPG = 0.0, maxMPG = 0.0, minPrice = 0.0, maxPrice = 0.0;

Double dMin = Double.MAX_VALUE;
Double dMax = Double.MIN_VALUE;
Double mpgMin = Double.MAX_VALUE;
Double mpgMax = Double.MIN_VALUE;
Double priceMin = Double.MAX_VALUE;
Double priceMax = Double.MIN_VALUE;

for (int i = 0; i < fillUps.length; i++) {
    if (fillUps[i].getDist() < dMin){
        dMin = fillUps[i].getDist();
        minDist = dMin;
    }     
    if (fillUps[i].getDist() > dMax) {
        dMax = fillUps[i].getDist();
        maxDist = dMax;
    }
    if (fillUps[i].getMilesPerGallon() < mpgMin) {
        mpgMin = fillUps[i].getMilesPerGallon();
        minMPG = mpgMin;
    }    
    if (fillUps[i].getMilesPerGallon() > mpgMax) {
        mpgMax = fillUps[i].getMilesPerGallon();
        maxMPG = mpgMax;
    }             
    if (fillUps[i].getPrice() < priceMin) {
        priceMin = fillUps[i].getPrice();
        minPrice = priceMin;
    }          
    if (fillUps[i].getPrice() > priceMax) {
        priceMax = fillUps[i].getPrice();
        maxPrice = priceMax;
    }
}

Answer 1

Each and every one of your if-statements can be simplified.

if (fillUps[i].getDist() > dMin){
    dMin = fillUps[i].getDist();
    minDist = dMin;
}

First of all, since you modify two variables at the same time, you can write it on the same line.

if (fillUps[i].getDist() > dMin){
    minDist = dMin = fillUps[i].getDist();
}

However, this is a sign that you might have too many variables, since minDist is the same as dMin. I would keep the variables of primitive type double and get rid of the not-primitive variables. Since Double.MAX_VALUE and Double.MIN_VALUE is a regular double, I don't see the need for the Double type. Just be sure to initialize your min/max variables properly (Double.MIN_VALUE or Double.MAX_VALUE).

And then, what you are doing is to always get the minimum of two values, so this can be simplified by using the Math.min method.

minDist = Math.min(fillUps[i].getDist(), minDist);

So each of your four-line if-statement segments can each become one line!

for (int i = 0; i < fillUps.length; i++) {
    minDist = Math.min(fillUps[i].getDist(), minDist);
    maxDist = Math.max(fillUps[i].getDist(), maxDist);
    minMPG = Math.min(fillUps[i].getMilesPerGallon(), minMPG);
    maxMPG = Math.max(fillUps[i].getMilesPerGallon(), maxMPG);
    minPrice = Math.min(fillUps[i].getPrice(), minPrice);
    maxPrice = Math.max(fillUps[i].getPrice(), maxPrice);
}

Answer 2

The code violates Tell, Don't Ask:

Procedural code gets information then makes decisions. Object-oriented code tells objects to do things. ~Alec Sharp

Before we get to that, consider:

fillUps[i].getDist() > dMin

Can be written:

fillUps[i].canTravel( dMin )

Similarly:

fillUps[i].getPrice() > priceMax

Can be written:

fillUps[i].priceExceeds( priceMax );

The important question remains unanswered: why does the code check against distance and price? If all you want to do is determine how much it will cost on a given tank of gas, then write:

float maxDistance = fillUps[i].calculateDistance();

If you want to know the price to fill the remainder of the tank, write:

float price = fillUps[i].calculateFillPrice();

Move the code that uses the variables into the class that has the variables.

You can also compare objects, rather than the internal variables. For example:

AnnualFuelUse fillUps[] // ...
AnnualFuelUse minPriceFillUp;

for (int i = 0; i < fillUps.length; i++) {
  if( fillUps[i].exceedsPrice( fillUps[i-1] ) ) {
    maxPriceFillUp = fillUps[i];
  }

  if( fillUps[i].exceedsDistance( fillUps[i-1] ) ) {
    maxDistanceFillUp = fillUps[i];
  }

  // ...
}

At the end of the loop you'll have objects that contain the desired information. With those objects you can then ask them to perform specific tasks. Then make the get accessor methods private.