1

I need to determine the average miles per hour for a trip and an object containing a list of trips. I'm thinking of two approaches. First approach, have a MphCalculator class that I would inject into each class such as

public class MphCalculator {

    public BigDecimal calculateMph(int durationSeconds, BigDecimal miles) {
        //calculate speed
    }

}

The second approach would be for each object to have their own reference to an MphCalculator class such as

public class SpeedCalculator {

    final int durationSeconds;
    final BigDecimal miles;

    public SpeedCalculator(int durationSeconds, BigDecimal miles) {
        this.durationSeconds = durationSeconds;
        this.miles = miles;
    }


    public BigDecimal calculateMph() {
        //calculate and store speed
    }

}

The second approach to me seems more Object Oriented, but it also seems as though I may be creating unnecessary objects. If anyone could provide some pros/cons then that'd be very helpful! Also, this is my first question, so if I could improve this post in any way let me know.

1
  • Why are you using BigDecimal? BigDecimal is useful for currency values where an exact decimal representation is required. But do you really need mph in exact decimal, possibly to a million decimal places? What's the difference between 50.5 mph and 50.500000001 mph? Answer: practically unmeasurable. Commented Apr 26, 2019 at 11:17

2 Answers 2

3

I'd pick the second approach, but not exactly the way you are proposing it. Here is the issue: You are introducing an object called SpeedCalculator, which is indicating that it is performing a calculation. But in fact, you are storing two values - duration and mileage together.

This is the principal remark I'm making. Let's look at that same pair - (duration, length) - as a value itself. It would be some TripMetrics object. This object wouldn't be a good place to hold logic, but it could easily expose public getters for duration and mileage. In that way, it would play the role of a pair which is designed to fit your domain - travelling.

Now, when put that way, we can ask the question - what is the state of this object? Look at its interface:

class TripMetrics {
    public Duration getDuration() { ... }
    public BigDecimal getDistance() { ... }
}

The point I'm trying to make is that we do not know the state. It is fully encapsulated. The state is something that makes it possible to obtain the duration and the distance. Then, why not adding another metric:

class TripMetrics {
    public Duration getDuration() { ... }
    public BigDecimal getDistance() { ... }
    public BigDecimal getAverageMilesPerHour() { ... }
}

When the design is put this way, it doesn't really matter which of these metrics have been calculated, and which have been stored in the object. The point is that this object has been designed to represent a concept - some values associated with the trip. There is no further need for an explicit calculator. (In fact, I have only renamed it to remove an expectation that there is some calculator around.)

2
  • Thank you very much! I guess the general idea is that when multiple objects need the same calculation, you can generally encapsulate that data in a new object that can perform the calculation. Commented Apr 21, 2019 at 14:49
  • Generally, yes, although that cannot be a universal answer. In many occasions, that approach would lead to explosion of methods in a class, and then you have to reach out for a different design. However, if you can erase distinction between stored data and calculated data, then it will be easier to consume objects. Commented Apr 21, 2019 at 15:49
0

There is a third option, which uses object-orientation even more. Objects are supposed to model "things", not actions, so "Calculator" objects do not usually fit.

If you have "Trips" and you need to calculate "AverageMph". The correct model would be:

public final class Trip {
    ...
    public AverageMph calculateAverageMph() {
        return new AverageMpg(distance, time);
    }
}

public final class AverageMph {
    // Whatever this needs to do, calculation is here
}

You didn't define any behavior for the AverageMph, so I left it blank. I assume it would be presented at some point, but whatever it is, the relevant method needs to be there, which would use the result of the calculation.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.