I need to call all steps one after another and update the same instance based on the outcome of each steps. And if there is an issue with any specific step, i need to set failStep field and stop executing the following step. I need some elegant way to handle this scenario. I tried map operation, but not sure how to stop the further execution if the step is failed.

package abc.service;

import lombok.Data;
import lombok.ToString;

import java.util.Optional;

public class Test {

    public static void main(String[] args) {
        Test test = new Test();
        test.func();
    }

    public void func() {
        Abc abc = new Abc();
        abc.setId("1234");

        Abc abcd = Optional.ofNullable(abc)
                .map(step1 -> getFirstStep(abc))
                .map(step1 -> getSecondStep(abc))
                .map(step1 -> getThirdStep(abc))
                .orElse(abc);
        System.out.println("Answer: "   abcd);
    }

    private Abc getFirstStep(Abc abc) {
        abc.setStep1("STEP1");
        return abc;
    }

    private Abc getSecondStep(Abc abc) {
        abc.setStep2("STEP2");
        abc.setFailStep("STEP2");
        return abc;
    }

    private Abc getThirdStep(Abc abc) {
        abc.setStep3("STEP3");
        return abc;
    }
}

@Data
@ToString
class Abc {
    private String id;
    private String step1;
    private String step2;
    private String step3;
    private String failStep;
}

One simple way is to add null check inside all step method.

private Abc getThirdStep(Abc abc) {
    if(abc.getFailStep() == null) {
        abc.setStep3("STEP3");
    }
    return abc;
}

Any suggestion to handle this better and elegant way.

CodePudding user response：

You can also do it the other way around, not creating Optional from abc but creating a sequence (list or whatever) from the functions. Something like this:

    public void func() {
        Abc abc = new Abc();
        abc.setId("1234");

        Function<Abc, Abc> f1 = input -> getFirstStep(input);
        Function<Abc, Abc> f2 = input -> getSecondStep(input);
        Function<Abc, Abc> f3 = input -> getThirdStep(input);

        Abc abcd = doStuff(abc, List.of(f1, f2, f2));

        System.out.println("Answer: "   abcd);
    }

    public Abc doStuff(Abc input, List<Function<Abc, Abc>> functions) {
        if (functions.isEmpty()) return input; //no more functions to apply
        else {
            final var function = functions.get(0);
            final var remainingFunctions = functions.subList(1, functions.size());
            final var abc = function.apply(input);
            if (abc.getFailStep() != null) {
                //Short circuit here
                return abc;
            } else {
                //continue applying the rest of the functions
                return doStuff(abc, remainingFunctions);
            }
        }
    }

This will terminate after step 2. Don't use exceptions to control the program flow.

CodePudding user response：

first of all, this a wrong usage of Optinal.map(). that utility should be only used if the given function (ex:getFirstStep(),getSecondStep() ) might return null value.

But if you still need to use Optinal here, there are two ways, but both have its own drawbacks.

1. Use custom RuntimeException and throw it from functions and catch it where you call the map method.
2. if a step failed, set that to the ABC object and return null, you can get the failure information from abc not from abcd , in this case map method will be executed 3 times, but no function will be executed after you return null.

CodePudding user response：

You can get the behavior you want by returning Optional so that an empty result signals a failure to the rest of a chain of flatMaps.

    public void func() {
        Abc abc = new Abc();
        abc.setId("1234");

        Abc abcd = Optional.ofNullable(abc)
                .flatMap(this::getFirstStep)
                .flatMap(this::getSecondStep)
                .flatMap(this::getThirdStep)
                .orElse(abc);
        System.out.println("Answer: "   abcd);
    }

    private static Optional<Abc> toReturn(Abc abc) {
      return abc.getFailStep() == null ? Optional.of(abc) : Optional.empty();
    }
    
    private Optional<Abc> getFirstStep(Abc abc) {
        abc.setStep1("STEP1");
        return toReturn(abc);
    }

    private Optional<Abc> getSecondStep(Abc abc) {
        abc.setStep2("STEP2");
        abc.setFailStep("STEP2");
        return toReturn(abc);
    }

    private Optional<Abc> getThirdStep(Abc abc) {
        abc.setStep3("STEP3");
        return toReturn(abc);
    }

However I strongly recommend against code like this. Everyone who reads it will waste five minutes figuring out how it works. If that's 50 people over time, you've wasted almost 4 hours plus all the effort of inventing this trickiness.

Just factor it into a flat function and use a series of "if" guards.

Abc createAbcAndApplySteps() {
  Abc abc = new Abc();
  abc.setId("1234");
  doFirstStep(abc);
  if (abc.getFailStep() != null) return abc;
  doSecondStep(abc);
  if (abc.getFailStep() != null) return abc;
  doThirdStep(abc);
  return abc;
}

Or better yet throw an exception at the point the error is flagged. They're in the language to make error handling clean.

  // Call 
  private static void fail(Abc abc, String failMsg) {
    abc.setFailStep(failMsg);
    throw AbcStepFailureException("Fail: "   failMsg);
  }

I guarantee you that any tech lead who knows their job will favor the third and second idioms over the first. "Elegance" is often just a fancy word for obscurity.