-4

I have the following class Subcontractor and a Utils Class named ResultUtils.

One of the Utils methods calculates the latest business year on a subcontractor. This method has to return an Optional Double, because it's possible that a subcontractor was newly added during the current year. So it has no statistics on the last two years. If a subcontractor was added one year before, it has statistics for the last business year, but not for the penultimate business year.

My problem is to chain the getLatestResult method correctly. In my example the ifPresentOrElse is wrong of course, because it cannot return any values.

How should I do that?

public class Subcontractor
{
    BusinessYear lastBusinessYear;
    BusinessYear penultimateBusinessYear;
    ...
}


public class ResultUtils
{
    public static Optional< Double > getLatestResult( Subcontractor )
    {
        // how to do it right here?
        getResult( Subcontractor.getLastBusinessYear()  )
            .ifPresentOrElse( r -> return r,
                             () -> return getResult( Subcontractor.getPenultimateBusinessYear() ) );
    }

    public static Optional< Double > getResult( BusinessYear businessYear )
    {
        if( businessYear == null ) return Optional.empty();

        // make some calculations
        return Optional.of( calculcatedResult );
    }
}
user2622344
  • 956
  • 1
  • 10
  • 26
  • `r -> return r` is invalid syntax for starters. It's either `r -> r`, `Function.identity()`, or `r -> { return r; }`. Seems you don't understand lambdas because I think `return` is not doing what you think it is in that context. – Michael Apr 16 '20 at 10:10
  • likewise `getLatestResult( Subcontractor )` there is no identifier – Michael Apr 16 '20 at 10:12
  • I presume `Subcontractor.getLastBusinessYear()` is not actually a static method, since `lastBusinessYear` is shown as a field. – Michael Apr 16 '20 at 10:12
  • `return getResult(Subcontractor.getLastBusinessYear()).or(() -> getResult(Subcontractor.getPenultimateBusinessYear()));` – Johannes Kuhn Apr 16 '20 at 10:14
  • @Naman The other question does not allow answers that are applicable for Java 9+ – Johannes Kuhn Apr 16 '20 at 10:34
  • @JohannesKuhn It does, you need to scroll further than just the marked answer. Read [this answer precisely](https://stackoverflow.com/a/34398891/1746118). I have still added more such Q&A to the duplicates for your reference. – Naman Apr 16 '20 at 12:53
  • 1
    Yeah, but this close got me thinking. The accepted answer does not mention `.or()`, and the answer you linked is 2 screens down. The answer is still valuable for people who use Java 8, but people who use a newer Java version are left out and keep writing the old complicated code. In the best case, the accepted answer would be edited to include the better style. – Johannes Kuhn Apr 16 '20 at 15:03
  • @JohannesKuhn my answer included java 9's `or` from start :) – Andrew Tobilko Apr 16 '20 at 16:59
  • It's about closing a question with a similar one but which restricts it's answer to an old version. The right answer to this question is to use `.or()` if you can. And if you can't, well, Java 8 is still in wide use, so an answer should take this into account. But look at the [question](https://stackoverflow.com/q/28514704/845414) this was closed as duplicate. Would you find the right answer? And should we teach people to write code, that while correct, is unnecessary complicated? – Johannes Kuhn Apr 17 '20 at 04:44

2 Answers2

4
public static Optional<Double> getLatestResult(Subcontractor subcontractor) {
    return getResult(subcontractor.getLastBusinessYear())
            .or(() -> getResult(subcontractor.getPenultimateBusinessYear()));
}

assuming you are running on Java 9+.

ifPresentOrElse unwraps the optional, while you still need to return one.

  1. Try getResult(subcontractor.getLastBusinessYear()).
  2. If it's present, return it.
  3. If it's absent, return another getResult(subcontractor.getPenultimateBusinessYear()).

A Java 8 solution would be

public static Optional<Double> getLatestResult(Subcontractor subcontractor) {
    Optional<Double> result = getResult(subcontractor.getLastBusinessYear());
    return result.isPresent() ? result : getResult(subcontractor.getPenultimateBusinessYear());
}
Andrew Tobilko
  • 48,120
  • 14
  • 91
  • 142
1

Let's fix the code:

public static Optional<Double> getLatestResult(Subcontractor sub) {
    return getResult(sub.getLastBusinessYear()).or(() -> getResult(sub.getPenultimateBusinessYear()));
}

You have two functions, both of them return an Optional. You only need the first with a value. Optional.or will invoke the Supplier if it does not have a value, otherwise it returns itself.

If you use Java 8, well...

public static Optional<Double> getLatestResult(Subcontractor sub) {
    Optional<Double> result = getResult(sub.getLastBusinessYear());
    if (result.isPresent())
        return result;
    return getResult(sub.getPenultimateBusinessYear());
}

Get the first result, check if it is present, if so, return it.
If not, return the second result.

Johannes Kuhn
  • 14,778
  • 4
  • 49
  • 73
  • 1
    Sidenote that `Optional.or()` was introduced in Java9 – Lino Apr 16 '20 at 10:18
  • 1
    Oh and your code doesn't work. `or` has the following signature: `or(Supplier extends Optional extends T>> supplier)` so you'd need to pass an optional not an actual value – Lino Apr 16 '20 at 10:19