Uploaded image for project: 'Groovy'
  1. Groovy
  2. GROOVY-7611

java.util.Optional should evaluate to false if empty

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4.5
    • Fix Version/s: 2.5.0-beta-1
    • Component/s: None
    • Labels:

      Description

      In the spirit of the rest of the Groovy truth semantics, I suggest that an empty java.util.Optional, which is essentially a stream-safe equivalent of null, should evaluate to false: asBoolean() should simply delegate to isPresent().

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user jwagenleitner opened a pull request:

          https://github.com/apache/groovy/pull/403

          GROOVY-7611: java.util.Optional should evaluate to false if empty

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/jwagenleitner/groovy GROOVY-7611-Optional-asBoolean

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/groovy/pull/403.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #403


          commit c680953b03af1d15e67a1e5a38040c90a6201490
          Author: John Wagenleitner <jwagenleitner@apache.org>
          Date: 2016-08-28T07:04:03Z

          GROOVY-7611: java.util.Optional should evaluate to false if empty


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/403 GROOVY-7611 : java.util.Optional should evaluate to false if empty You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy GROOVY-7611 -Optional-asBoolean Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/403.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #403 commit c680953b03af1d15e67a1e5a38040c90a6201490 Author: John Wagenleitner <jwagenleitner@apache.org> Date: 2016-08-28T07:04:03Z GROOVY-7611 : java.util.Optional should evaluate to false if empty
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user blackdrag commented on a diff in the pull request:

          https://github.com/apache/groovy/pull/403#discussion_r76537642

          — Diff: src/main/org/codehaus/groovy/runtime/DefaultGroovyMethods.java —
          @@ -10563,13 +10563,21 @@ private void prepare() {
          /**

          • Coerce an object instance to a boolean value.
          • An object is coerced to true if it's not null, to false if it is null.
            + * One exception to this is in the case of a non-null {@code java.util.Optional}

            + * in which case the result of the

            {@code isPresent}

            method is returned.
            *

          • @param object the object to coerce
          • @return the boolean value
          • @since 1.7.0
            */
            public static boolean asBoolean(Object object) {
          • return object != null;
            + if (object == null) { + return false; + }

            + if ("java.util.Optional".equals(object.getClass().getName())) {

              • End diff –

          just a very small thing.. could you add a todo to move this into a jvm8 module once we decided to make that our minimum jdk for the build?

          Show
          githubbot ASF GitHub Bot added a comment - Github user blackdrag commented on a diff in the pull request: https://github.com/apache/groovy/pull/403#discussion_r76537642 — Diff: src/main/org/codehaus/groovy/runtime/DefaultGroovyMethods.java — @@ -10563,13 +10563,21 @@ private void prepare() { /** Coerce an object instance to a boolean value. An object is coerced to true if it's not null, to false if it is null. + * One exception to this is in the case of a non-null {@code java.util.Optional} + * in which case the result of the {@code isPresent} method is returned. * @param object the object to coerce @return the boolean value @since 1.7.0 */ public static boolean asBoolean(Object object) { return object != null; + if (object == null) { + return false; + } + if ("java.util.Optional".equals(object.getClass().getName())) { End diff – just a very small thing.. could you add a todo to move this into a jvm8 module once we decided to make that our minimum jdk for the build?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jwagenleitner commented on a diff in the pull request:

          https://github.com/apache/groovy/pull/403#discussion_r76540119

          — Diff: src/main/org/codehaus/groovy/runtime/DefaultGroovyMethods.java —
          @@ -10563,13 +10563,21 @@ private void prepare() {
          /**

          • Coerce an object instance to a boolean value.
          • An object is coerced to true if it's not null, to false if it is null.
            + * One exception to this is in the case of a non-null {@code java.util.Optional}

            + * in which case the result of the

            {@code isPresent}

            method is returned.
            *

          • @param object the object to coerce
          • @return the boolean value
          • @since 1.7.0
            */
            public static boolean asBoolean(Object object) {
          • return object != null;
            + if (object == null) { + return false; + }

            + if ("java.util.Optional".equals(object.getClass().getName())) {

              • End diff –

          Good suggestion, I've added the todo comment.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jwagenleitner commented on a diff in the pull request: https://github.com/apache/groovy/pull/403#discussion_r76540119 — Diff: src/main/org/codehaus/groovy/runtime/DefaultGroovyMethods.java — @@ -10563,13 +10563,21 @@ private void prepare() { /** Coerce an object instance to a boolean value. An object is coerced to true if it's not null, to false if it is null. + * One exception to this is in the case of a non-null {@code java.util.Optional} + * in which case the result of the {@code isPresent} method is returned. * @param object the object to coerce @return the boolean value @since 1.7.0 */ public static boolean asBoolean(Object object) { return object != null; + if (object == null) { + return false; + } + if ("java.util.Optional".equals(object.getClass().getName())) { End diff – Good suggestion, I've added the todo comment.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jwagenleitner closed the pull request at:

          https://github.com/apache/groovy/pull/403

          Show
          githubbot ASF GitHub Bot added a comment - Github user jwagenleitner closed the pull request at: https://github.com/apache/groovy/pull/403
          Hide
          jwagenleitner John Wagenleitner added a comment -

          Closed PR# 403 due to some performance concerns with it's implementation. It was suggested to have a dedicated jdk8 module, so will wait until 8 is made the min for build.

          Show
          jwagenleitner John Wagenleitner added a comment - Closed PR# 403 due to some performance concerns with it's implementation. It was suggested to have a dedicated jdk8 module, so will wait until 8 is made the min for build.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user jwagenleitner opened a pull request:

          https://github.com/apache/groovy/pull/545

          GROOVY-7611: java.util.Optional should evaluate to false if empty (Java8 VMPlugin)

          Target for this would be `2_5_X` and above. For `2_5_X` and `2_6_X` it would require that the release process build with JDK 8 so would need to backport some of the build checks for Java version to those branches.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/jwagenleitner/groovy 7611-Optional

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/groovy/pull/545.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #545


          commit 720dced06b1ec447d77b00550b8b299dfe3e18d9
          Author: John Wagenleitner <jwagenleitner@apache.org>
          Date: 2017-05-21T17:16:06Z

          GROOVY-7611: java.util.Optional should evaluate to false if empty


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user jwagenleitner opened a pull request: https://github.com/apache/groovy/pull/545 GROOVY-7611 : java.util.Optional should evaluate to false if empty (Java8 VMPlugin) Target for this would be `2_5_X` and above. For `2_5_X` and `2_6_X` it would require that the release process build with JDK 8 so would need to backport some of the build checks for Java version to those branches. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jwagenleitner/groovy 7611-Optional Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/545.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #545 commit 720dced06b1ec447d77b00550b8b299dfe3e18d9 Author: John Wagenleitner <jwagenleitner@apache.org> Date: 2017-05-21T17:16:06Z GROOVY-7611 : java.util.Optional should evaluate to false if empty
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/groovy/pull/545

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/groovy/pull/545
          Hide
          jwagenleitner John Wagenleitner added a comment -

          Thanks for improvement request.

          Show
          jwagenleitner John Wagenleitner added a comment - Thanks for improvement request.
          Hide
          paulk Paul King added a comment - - edited

          Marked as a breaking change for anyone relying on the old behavior.

          If you were working around the old behavior by using this long-hand form, i.e.:

          def empty = Optional.empty()
          def foo = Optional.of('foo')
          assert foo.isPresent() ? 1 : -1 == 1
          assert empty.isPresent() ? 1 : -1 == -1
          

          you can leave the code as is or change it to the shorter equivalent:

          assert foo ? 1 : -1 == 1
          assert empty ? 1 : -1 == -1
          
          Show
          paulk Paul King added a comment - - edited Marked as a breaking change for anyone relying on the old behavior. If you were working around the old behavior by using this long-hand form, i.e.: def empty = Optional.empty() def foo = Optional.of('foo') assert foo.isPresent() ? 1 : -1 == 1 assert empty.isPresent() ? 1 : -1 == -1 you can leave the code as is or change it to the shorter equivalent: assert foo ? 1 : -1 == 1 assert empty ? 1 : -1 == -1

            People

            • Assignee:
              jwagenleitner John Wagenleitner
              Reporter:
              chrylis Christopher Smith
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development