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

first pr of Groovy-Optional #420

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

uehaj
Copy link
Contributor

@uehaj uehaj commented May 18, 2014

Last year I proposed a way of transparent handling of java.util.Optional.

http://groovy.329449.n5.nabble.com/Groovier-way-of-handling-Java8-s-java-util-Optional-td5717544.html#a5717561
https://github.com/uehaj/groovy-optional

I think this is very groovy way to handle Optional value, although Optional itself might not be good idea compared to safe navigation, anyway.
I imagine you groovy development team are tackling Java8 aware GDK methods?

So I rewrite above proof of concept code to actual PR to current Groovy
implemented as a GDK method java.util.Optional.methodMissing().

I hope reviewing this, and please advice me about anything to do.
(for example where is the right branch to rebase?)
Probably it is needed to add custom type checking rule to avoid STC errors
which comes from use of Optional.methodMissing.

Best regards,

@blackdrag
Copy link
Member

The dsadvantage of using missingMehod is of course, that any method defined on Optional cannot be called that way. I am worried about map, flatMap, filter, toString, equals

@uehaj
Copy link
Contributor Author

uehaj commented May 19, 2014

Thanks your comment.
methodMissing captures only call to method which is not exists on Optional,
So those methods are called exactly with the same way of Java and it works expectedly.
I added test cases:

    void testMap() {
        expect:
        Optional.of("abc").map{it.toUpperCase()} == Optional.of("ABC")
    }

    void testFlatMap() {
        def divide = {it == 0 ? Optional.empty() : Optional.of(3.0/it) }
        expect:
        Optional.of(3.0).flatMap(divide) != Optional.of(1.0) // equals i not dispatched to target.
        Optional.of(3.0).flatMap(divide).get() == Optional.of(1.0).get()
        Optional.of(0).flatMap(divide) == Optional.empty()
    }

    void testFilter() {
        expect:
        Optional.of(3).filter { it % 2 == 0 } == Optional.empty()
        Optional.of(3).filter { it % 2 == 1 } == Optional.of(3)
    }

those behavior is exactly same as Java so upper compatibility is kept perfectly.

But in the case of toString() /equals() / hashCode() might be regarded as a problem.
java.util.Optional is of course Java's API class, so in the GDK, it is impossible or
not recommended to change the semantics of Java API for compatibility reason isn't it?

So I think we have no choice to override the meaning of Optional's methods
-- equals() and toString() and hashCode() -- because we expect the converted
Groovy code from Java has same (upper compatible) meaning of java code.

@blackdrag
Copy link
Member

what I did mean was more something like this:

[1l,2l].stream().filter {it>1} 

will return a Stream of Long with 2 as single element,

Optional.of([1l,2l].stream()).filter {it>1}

will not work at all, because it would compare the number 1 with a Stream of Longs. It looks a bit constructed with a Stream, but it is to be expected that there are many more tpyes that support that in the future.

What I am saying is that wrapping a value in an Optional and then have the wrapper act transparently lke the object inside is not going to work. And even if we fix hashcode and equals for us, the moment you place it in a collection all bets are off again.

I won't say this pull request is a bad idea, not at all! But first we should clarify what the target of this is and the see what we need to get there

@uehaj
Copy link
Contributor Author

uehaj commented May 19, 2014

I get the point.
I'd like to try to describe the purpose and target use cases.

Optional is a idea comes from Scala or some FP area(haskell, ocaml,..)
and those languages have pattern matching language construct which
ordinary used as a simple way to extract Optional/Option/Maybe value
but Java and Groovy don't have pattern match now.

The problem of Optional is, to get the value from Optional,
You have to do isPresent() check instead of null check.

name = Optional.of("who") or Optional.empty()
if(name.isPresent()){
  System.out.println( name.get() );
}

if you have 2 values, and wrap up to Optional after operation, the code would be like:

Optional<String> fullName = (lastName.isPresent() && firstName.isPresent()) ? 
        Optional.of(lastName.get() + " " + firstName.get())) : 
        Optional.empty();

This is almost as bad as null handling.
so we have flatMap.

Optional<String> fullName = 
  lastName.flatMap(ln -> 
  firstName.flatMap(fn -> 
    Optional.of(ln + " " + fn))));

but this still verbose, Especially in Groovy, because we have safe navigation already.
with safe navigation and null, you can write:

println(fullName?.plus(' ')?.plus(lastName))

In Scala, for-comprehension (monad comprehension) make more simple, but Groovy don't have now.

To sum up, there is no good enough language support in Java to extract Optional value and operate it and wrap-up again after operation.

By this proposal we can write simple-and intuitive way.

Optional<String> fullName = lastName + firstName

But this works just under restriction which your pointed.

As another idea, how about expand the meaning of "?. " on Optional?

  • [Optional Value] ?. property means [Optional Value].isPresant() ? [Optional Value].get().property : Optional.empty()
  • [Optional Value] ?. method(..) means [Optional Value].isPresant() ? [Optional Value].get().method(..) : Optional.empty()

@blackdrag
Copy link
Member

sorry, for the really long delay... using "?." is interesting, but your original problem was

Optional<String> fullName = (lastName.isPresent() && firstName.isPresent()) ? 
    Optional.of(lastName.get() + " " + firstName.get())) : 
    Optional.empty();

and there it won't help.

@uehaj
Copy link
Contributor Author

uehaj commented Dec 25, 2014

thank you comment again. You are right.
That was wrong. meaning of Optional-aware "?." is not so trivial.

I think ?. is converted depends on weather the type of method parameter is Optional or not.
When under Static Type Check, you can generate like following:

// lastName?.plus(" ")
Optional<String> tmp =
    lastName.flatMap { s1 -> Optional.of(s1.plus(" ")) }
assert tmp.get() =="lastname "

// lastName?.plus(" ")?.plus(firstName)
// converted to(under STC):
Optional<String> fullName1 =
    lastName.
    flatMap { s1 -> Optional.of(s1.plus(" ")) }.
    flatMap { s2 -> firstName.flatMap{s3->Optional.of(s2.plus(s3))}  }
assert fullName1.get()=="lastname firstName"

without STC, dynamic type checking with instanceof call is required.
for example,

// lastName?.plus(" ")?.plus(firstName)
// converted to(without STC):
Optional<String> fullName3 =
           ((lastName instanceof Optional) ? lastName: Optional.of(lastName)).
           flatMap { s1 -> def arg = " "
               if (arg instanceof Optional) { return arg.flatMap{ s2 -> Optional.of(s1.plus(s2)) } }
               else { return Optional.of(s1.plus(arg)) } }.
           flatMap { s1 -> def arg = firstName
               if (arg instanceof Optional) { return arg.flatMap{ s2 -> Optional.of(s1.plus(s2)) } }
               else { return Optional.of(s1.plus(arg)) } }
assert fullName3.get()=="lastname firstName"

In addition, null check should be done in actual code generation.

@blackdrag
Copy link
Member

but you see that you cannot write lastName + " " + firstName to get this result? It is because there is no way to express a "?." for operators. It is already the same for safe null navigation, but you while you use null-safe navigation only in a few places normally, you are forced to use optionals all the time, once you start using them. In that optionals are a very intrusive element.

That starts for me the question if a syntax element you have to use all the time for this is the right thing. Instead maybe it should go deeper and become part of Groovy's internals. This idea goes a lot more in the direction of your initial idea. But what I have in mind is much more complex. Instead of having some kind of helper type to do dynamic dispatch with and being lost in static compilation, How about making optional a "wrapper type". That means if the runtime detects an optional, it reacts internally different to it having a value present or not. Then a+b could be transparently done with optionals.

To give an example of what I am thinking here... if you make a method call on receiver, then the runtime will check if the receiver is null, and act different according to that. Of course Optional would be done slightly different... and potentially a lot of places have to change for this...but this is where I see the maximum use...

Do you agree?

@mperry
Copy link

mperry commented Dec 26, 2014

I commented on this and then accidentally deleted it, so I will try again.

@uehaj You want to transform a sequence of safe navigation to a sequence of flatMap calls, with the last call being a map. This will remove the need for Optional.of. Your example of:

lastName?.plus(" ")?.plus(firstName)

assumes lastName is optional, but firstName is mandatory. Let's assume lastName is also optional and the space is mandatory. The code is then:

lastName.flatMap { ln -> firstName.map { fn -> fn + " " + ln } }

or if " " is optional then:

lastName.flatMap { fn -> space.flatMap -> { s -> lastName.map { ln -> fn + s + ln } } }

I wrote a dynamically typed method to do this sequence of calls using missingMethod calls so you can write:

foreach {
  fn << firstName
  s << space
  ln << lastName
  yield { fn + s + ln }
}

I intended to rewrite using a macro, but have not gotten around to it. With a macro, this could be typechecked statically. Without a macro (for static typing) you need to ensure the values are monads (so they have flatMap and map) or can be transformed to monads. Scala and Haskell do this using implicits and typeclasses. @paulk-asert has made progress towards implicit conversion for assignment statements. It would be good if this could be extended to general expressions.

@uehaj
Copy link
Contributor Author

uehaj commented Dec 26, 2014

Hi brackdrag,
First of all, I believe my first proposal can be implemented without dynamic method dispatch, under STC , by using custom type checker(type checking extension) which handles methodNotFound event. Of course standard type checker can be changed to do it. So we don't have to lose static compilation. But this is not the point.

I guess your "wrapper type" of Optional handling is done under the similar way where primitive types handling in Groovy isn't it? Java's primitive types are converted to Boxed type versa-visa at run time.
If this guess is right, It might works well. Because Optional is Immutable type, referencing target cant be changed throughout lifetime of Optional instance. Probably equals() and hashCode() is forwarded to the wrapped target of Optional? wow, it's nice idea. I agreed.

But it could be hard work. Not only around Java method call, you have to wrap/retrieve operation at all of the retrieve/store operation for the containers which have Optional value, like List() or array of Optional, .etc.
thx

@uehaj
Copy link
Contributor Author

uehaj commented Dec 26, 2014

Hi, mperry, thanks for comment.
I also implemented "type class in groovy" aka Groovyz which introduce implicit parameter based type class. I wrote an article about it (but in Japanese, sorry)

http://uehaj.hatenablog.com/entry/2014/12/24/030136
Learning Type Class thought implementing it in Groovy
POC source code: https://github.com/uehaj/groovyz

best regards,

@mperry
Copy link

mperry commented Dec 26, 2014

@uehaj I found the auto translation very hard to read. I would be interested in your explanation of the custom type checker.

@uehaj
Copy link
Contributor Author

uehaj commented Dec 28, 2014

Hi @mperry I wrote document: https://github.com/uehaj/groovyz if you prefer

@uehaj
Copy link
Contributor Author

uehaj commented Feb 22, 2015

FYI:
Rust people are considering to adopt null safe looking operation (?.) on IoResult (corresponding to Optional of java8).

https://github.com/rust-lang/rfcs/blob/master/text/0517-io-os-reform.md#the-error-chaining-pattern
The error chaining pattern:
File::open(some_path)?.read_to_end()

traneHead pushed a commit to traneHead/groovy-core that referenced this pull request Jul 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants