Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Match.run(cases) syntax #1216

Closed
danieldietrich opened this issue Mar 22, 2016 · 4 comments
Closed

Add Match.run(cases) syntax #1216

danieldietrich opened this issue Mar 22, 2016 · 4 comments

Comments

@danieldietrich
Copy link
Contributor

Timo Nentwig figured out that the following Match example (from the blog post Pattern Matching Essentials) does not work:

Match(arg).of(  
    Case(isIn("-h", "--help"), run(this::displayHelp)),
    Case(isIn("-v", "--version"), run(this::displayVersion)),
    Case($(), run(() -> {
        throw new IllegalArgumentException(arg);
    }))
);

All cases are evaluated eagerly, the last throws and the of() method is never reached.

Reason/Bug: the API.run(Runnable) method evaluates eagerly.

I'm not sure how to process further:

Solution 1) remove the run() method

Solution 2) lift the Runnable to a return value in order to fit Case(..., R):

```java
public static Runnable run(Runnable unit) {
    return unit;
}

@Test
public void shouldRunUnitOfWork() {
    Match("-h").of(
            Case(isIn("-h", "--help"), run(this::displayHelp)),
            Case(isIn("-v", "--version"), run(this::displayVersion)),
            Case($(), run(() -> {
                throw new IllegalArgumentException();
            }))
    ).run(); // <--- additional call needed
}

void displayHelp() {
    System.out.println("Help");
}

void displayVersion() {
    System.out.println("Version");
}
```
@danieldietrich danieldietrich self-assigned this Mar 22, 2016
@danieldietrich danieldietrich added this to the 2.1.0 milestone Mar 22, 2016
This was referenced Mar 23, 2016
@danieldietrich
Copy link
Contributor Author

We suggested in #1218 to drop run(Runnable) in favor of:

    static <T> Function<T, Void> run(Consumer<? super T> consumer) {
        return t -> {
            consumer.accept(t);
            return null;
        };
    }

    static <T1, T2> BiFunction<T1, T2, Void> run(BiConsumer<? super T1, ? super T2> consumer) {
        return (t1, t2) -> {
            consumer.accept(t1, t2);
            return null;
        };
    }

    void test() {
        Match(new Object()).of(
                Case(instanceOf(Integer.class), run(i -> System.out.println(i))),
                Case(instanceOf(Double.class), run(d -> System.out.println(d))),
                Case($(), run(ignored -> { throw new NumberFormatException(); }))
        );
    }

This needs to be tested for all Consumers to ensure that there are no ambiguities.

Where to place the Consumers? As top-level interfaces in package javaslang or as within javaslang.API?

@danieldietrich
Copy link
Contributor Author

This is not a bug, the run() method has to be used in another way.

We must not use run() as eagerly evaluated return value:

// R Case(Pattern0<T>, R)
Case($(o -> false), run(...))

In the above example run(...) is evaluated before it is checked if the pattern matches. Instead we need to define a proper function that handles a value if the pattern matches:

// R Case(Pattern0<T>, Function<T, R>)
Case($(o -> false), o -> run(...))

In the above example run(...) is evaluated if and only if the pattern matches.


Because we cannot provide a Case API that forces developers to use the right variant, the run() API can be considered as unsafe. run should be deprecated

How can the Match API evolve in the way that we are able to run code in a Match Case (in a concise way) while staying safe?

We need an API that makes clear that side-effects take place, i.e. that no value is returned. This can be accomplished by adding

Match(obj).run(cases)

where we have to ensure that only cases are used that result in Runnables. I think of

Match(obj).run(
    Case(Pattern, () -> ...) // Runnable as return value
    Case(Pattern, (t1, ..., tn) -> () -> {}) // curried form of a Runnable as function
)

I need to check if this or s.th. similar is posstible.

@danieldietrich danieldietrich changed the title Case(..., run(Runnable)) does evaluate Runnable eagerly Add Match.run(cases) syntax Mar 23, 2016
@danieldietrich
Copy link
Contributor Author

This works well, run is not evaluated eagerly:

@Test
public void shouldRunUnitOfWork() {

    class OuterWorld {

        String effect = null;

        void displayHelp() {
            effect = "help";
        }

        void displayVersion() {
            effect = "version";
        }
    }

    final OuterWorld outerWorld = new OuterWorld();

    Match("-v").of(
            Case(isIn("-h", "--help"), o -> run(outerWorld::displayHelp)),
            Case(isIn("-v", "--version"), o -> run(outerWorld::displayVersion)),
            Case($(), o -> { throw new IllegalArgumentException(); })
    );

    assertThat(outerWorld.effect).isEqualTo("version");
}

@Test
public void shouldRunWithInferredArguments() {

    class OuterWorld {

        Number effect = null;

        void writeInt(int i) {
            effect = i;
        }

        void writeDouble(double d) {
            effect = d;
        }
    }

    final OuterWorld outerWorld = new OuterWorld();
    final Object obj = .1d;

    Match(obj).of(
            Case(instanceOf(Integer.class), i -> run(() -> outerWorld.writeInt(i))),
            Case(instanceOf(Double.class), d -> run(() -> outerWorld.writeDouble(d))),
            Case($(), o -> { throw new NumberFormatException(); })
    );

    assertThat(outerWorld.effect).isEqualTo(.1d);
}

@danieldietrich danieldietrich modified the milestones: 2.2.0, 2.1.0 Aug 26, 2016
@danieldietrich danieldietrich modified the milestones: 2.1.0, 2.2.0 Oct 23, 2016
@danieldietrich danieldietrich modified the milestones: 3.0.0, 2.1.0 Mar 5, 2017
@danieldietrich danieldietrich modified the milestones: vavr-1.0.0, vavr-1.1.0 Nov 26, 2017
@danieldietrich
Copy link
Contributor Author

We will not push side-effecting programming.

@danieldietrich danieldietrich removed this from the next milestone Jan 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant