2

Let's assume I have got a List of Flight Objects

public class Flight {
    private int passengers;
    private int price
    ...

    //getters and Setters
}

List<Flight> flightList = new ArrayList<Flight>();

Now i need to accumulate price per passenger and the Price, because I have to be able to proceed both informations later on. so I would create two methods:

public int calculatePrice(List<Flight> flightList) {
    int price = 0;
    for (Flight flight : flightList) {
        price = price + flight.getPrice();
    }
    return price;
}

public int calculatePricePerPassengers(List<Flight> flightList) {
    int pricePerPassenger = 0;
    for (Flight flight : flightList) {
        pricePerPassenger = (pricePerPassenger) + (flight.getPrice() / flight.getPassgers());
    }
    return pricePerPassenger;
}

I have got like 4-5 methods of the same type. I am not sure whether there is too much redundancy, because I call the for loop 4-5 Times and I could easily do all the operations in one for loop, but that would have the effect of multiple return values. So is this the appropriate way ? (just a dummy example)

Vasan
  • 4,810
  • 4
  • 20
  • 39
  • I'm sorry - but it's not a duplication, and it's not about returning multiple values. It's more about redundancy and caching which might be a solution here. The OP mentioned multiple return values only as a side effect of the approach he thought to take. – SHG Feb 07 '18 at 17:31
  • create an object that has all fields you want to return and use that object in the loop and return it will all info you will need later. – isaace Feb 07 '18 at 17:33
  • you may iterate once and collect all the need information – Andrew Tobilko Feb 07 '18 at 17:34
  • One solution will be to have one function with one loop in which you calculate all the possible values that you want. As return value, you'll return only the one requested from the user, and all the rest of the value you're gonna save, but if the data is changing you'll have to update this cached data, so have some flag of "needsUpdate" and do it according to it. – SHG Feb 07 '18 at 17:41
  • Just add the fields that you need calculate to Flight class, then do it in a single loop. – Karpinski Feb 07 '18 at 17:43

3 Answers3

5

You can use map for that or also you can warp all the fields you want to return in a class. Here I am giving the example using Map.

public static Map<String, Integer> calculatePrice(List<Flight> flightList) {
    Map<String, Integer> priceMap = new HashMap<>();
    int price = 0;
    int pricePerPassenger = 0;
    for (Flight flight : flightList) {
        price = price + flight.getPrice();
        pricePerPassenger = (pricePerPassenger) + (flight.getPrice() / flight.getPassgers());
    }
    priceMap.put("price", price);
    priceMap.put("pricePerPassenger", pricePerPassenger);
    return priceMap;
}

EDIT: Example using a wrapper class (DTO).

    public static FligtPriceDTO calculatePrice(List<Flight> flightList) {
        FligtPriceDTO dto = new FligtPriceDTO();
        int price = 0;
        int pricePerPassenger = 0;
        for (Flight flight : flightList) {
            price = price + flight.getPrice();
            pricePerPassenger = (pricePerPassenger) + (flight.getPrice() / flight.getPassgers());
        }
        dto.setPrice(price);
        dto.setPricePerPassenger(pricePerPassenger);
        return dto;
    }
}

class FligtPriceDTO {
    private int price;
    private int pricePerPassenger;

    public int getPrice() {
        return price;
    }

    public void setPrice(int price) {
        this.price = price;
    }

    public int getPricePerPassenger() {
        return pricePerPassenger;
    }

    public void setPricePerPassenger(int pricePerPassenger) {
        this.pricePerPassenger = pricePerPassenger;
    }

}
Amit Bera
  • 7,075
  • 1
  • 19
  • 42
  • 3
    While plausible, I'd recommend a more strongly typed object/DTO – Taylor Feb 07 '18 at 17:41
  • Right, I have edited and added an example using DTO. Thanks! – Amit Bera Feb 07 '18 at 17:44
  • 1
    Generally always go for the second option, using a object. As there could be a case with the `Map` where a key could be the same value as one previously stored and effectively overwriting it the previous key with a new value. – Tom C Feb 07 '18 at 19:00
1

You could wrap those two values into a new array:

public int[] calculatePrices(List<Flight> flightList) {

    int totalPrice = 0;
    int pricePerCustomer = 0;

    for (Flight flight : flightList) {
        totalPrice += flight.getPrice();
        pricePerCustomer += (flight.getPrice() / flight.getPassgers());
    }

    return new int[] { totalPrice, pricePerCustomer };
}
Antony H
  • 40
  • 4
0

It all comes down to what are the functionalities that you want to provide to the user. So if the user expects to be able to calculate both prices individually then I do not see any other way around what you have. But if you can present both prices at once, then you can eliminate one of the methods and loop only once trough your Flight list and return a composition of both prices. ex:

public static Pair[] calculatePrices(List<Flight> flightList)
{
     int pricePerFlight= 0;
     int pricePerPassenger = 0;
     for(Flight flight : flightList)
    {
       pricePerFlight = pricePerFlight + flight.getPrice();
       pricePerPassenger =   pricePerPassenger  +(flight.getPrice()/flight.getPassgers());

     }
    return new Pair[] {new Pair<>("pricePerFlight", pricePerFlight), new Pair<>("pricePerPassenger", pricePerPassenger )};
}
MAV
  • 26
  • 2