groovy
  1. groovy
  2. GROOVY-4899

findResults object/collection/map enhancement patch (CLONED from findResult)

    Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.8.1
    • Component/s: None
    • Labels:
      None
    • Testcase included:
      yes
    • Patch Submitted:
      Yes
    • Number of attachments :
      2

      Description


      CLONED!! Read first comment rather than entire description.

      Groovy find and findAll take the result of the passed in closure and use groovy truth to either find the first element in the collection/map or filter the collection/map.

      There are many situations situations where what you actually want to find or collect is the result of the closure, rather than test original elements and then reprocessing them all again.

      The attached patch (with javadocs & unit tests) adds 2 methods to groovy objects, collections, and maps; enhanced versions of the existing groovy find and findAll methods:

      <object/collection/map>.findResult(closure)
      <object/collection/map>.findAllResults(closure)
      

      findResult and findAllresults are valuable for shortening/clarifying code in many situations, but they're the most valuable when the closure calls a relatively expensive method (or one that isn't idempotent). When calling the method again with the results of a find/findAll is impractical.

      I had a discussion today on twitter with Robert Fletcher (http://twitter.com/rfletcherEW/status/16410798707) that spurred this patch, though there have been many times in the past that I've wanted this functionality and ended up writing the 4+ lines of boilerplate code to work around it being missing.

      Details:

      The object/collection version of findAll iterates through the object and passes each value to the closure. The first closure result that is non-null (including false) will short circuit evaluation and return the value, example:

      // works on lists/collections
      assert "RAB" == ["foo", "bar", "baz"].findResult {
          def uCaseReversed = it.reverse().toUpperCase()  // proxy for expensive method that we don't want to do more often than necessary
          if (uCaseReversed ==~ /..B/) return uCaseReversed
      }
      
      // and objects with iterators like ranges
      assert "Found 4" == (1..10).findResult {
          if (it > 3) return "Found $it"
      }
      

      The map version of findResult is slightly different as what it returns is a Map.Entry with the key equal to the key of the map entry that generated a non-null value from the closure:

      // closure has 0/1 parameters, it passes the current Map.Entry in as it iterates
      [a: 1, b: 2, c: 3, d: 4].findResult { 
          if (it.value > 2) return "I found ${it.key}:${it.value}"
      }.with {
          assert "c" == it.key
          assert "I found c:3" == it.value
      }
      
      // closure has 2 parameters, it passes the key and value in as it iterates
      [a: 1, b: 2, c: 3, d: 4].findResult { key, value ->
          if (value > 2) return "I found ${key}:${value}"
      }.with {
          assert "c" == it.key
          assert "I found c:3" == it.value
      }
      

      findAllResults will spin through the whole iterator, but will collect only those results from the closure that are non-null, so it can both collect and filter in one step:

      assert ["Found 4", "Found 5"] == (1..5).findAllResults {
          if (it > 3) return "Found $it"
      }
      

      The map version returns a map that contains the keys that had a non-null result with the result of the closure:

      def origMap = [a:1, b:3, c:5]
      origMap.findAllResults { if (it.value >= 3) return "Found ${it.key}:${it.value}" }.with { mappedResults ->
          assert 2 == mappedResults.size()
          assert "Found b:3" == mappedResults.b
          assert "Found c:5" == mappedResults.c
      }
      

      Other groovy iterators (like collect and inject) are not able to easily fulfill this role. Collect will not do any filtering, but instead returns a full list with nulls that need to be removed. Inject can sort of be made to fill this role, but it's often more work than it's worth as you need to manage the list creation/appending and the additional closure parameter. There isn't an equivalent of collect/inject on Maps.

      I'd love it if this could be added to 1.7.4, but 1.8 would work too .

        Issue Links

          Activity

          Hide
          Paul King added a comment - - edited

          There seems to be interest in other variations of the original request. Attached is a patch containing two enhancements.

          The first is for findResults which is basically collect but automatically removing null values:

          def list = [1,2,3]
          def result = list.findResults { it > 1 ? "Found $it" : null }
          assert result == ["Found 2", "Found 3"]
          

          The second is for joinCollected which is like "flatmap" from Haskell and "mapcat" from Clojure:

          def list = [1,2,3]
          def result = list.joinCollected { it > 1 ? ["Found $it"] : [] }
          assert result == ["Found 2", "Found 3"]
          
          def nums = 1..5
          def squaresAndCubesOfOdds = nums.joinCollected{ it % 2 ? [it**2, it**3] : [] }
          assert squaresAndCubesOfOdds == [1, 1, 9, 27, 25, 125]
          

          There is still some debate about whether aliases should be provided for these but attaching a potential patch here that aligns (mostly) with current naming to capture one possible solution. Aliases could be done as a subsequent activity.

          Show
          Paul King added a comment - - edited There seems to be interest in other variations of the original request. Attached is a patch containing two enhancements. The first is for findResults which is basically collect but automatically removing null values: def list = [1,2,3] def result = list.findResults { it > 1 ? "Found $it" : null } assert result == [ "Found 2" , "Found 3" ] The second is for joinCollected which is like "flatmap" from Haskell and "mapcat" from Clojure: def list = [1,2,3] def result = list.joinCollected { it > 1 ? [ "Found $it" ] : [] } assert result == [ "Found 2" , "Found 3" ] def nums = 1..5 def squaresAndCubesOfOdds = nums.joinCollected{ it % 2 ? [it**2, it**3] : [] } assert squaresAndCubesOfOdds == [1, 1, 9, 27, 25, 125] There is still some debate about whether aliases should be provided for these but attaching a potential patch here that aligns (mostly) with current naming to capture one possible solution. Aliases could be done as a subsequent activity.
          Hide
          Paul King added a comment - - edited

          The name joinCollected was given slight preference to sumCollected. The assumption is that we would add a join method on collections. It would act like join for Strings - basically an alias for sum if no parameter was given but would concatenate with a 'joining' element if supplied with a parameter, e.g.:

          def pets = ['ant', 'bee']
          def food = ['bread', 'honey']
          assert [pets, food].join() == ['ant', 'bee', 'bread', 'honey']
          assert [pets, food].join('eats') == ['ant', 'bee', 'eats', 'bread', 'honey']
          

          This conflicts somewhat with existing DGM join usage, so we are still thinking about our options here.

          Show
          Paul King added a comment - - edited The name joinCollected was given slight preference to sumCollected . The assumption is that we would add a join method on collections. It would act like join for Strings - basically an alias for sum if no parameter was given but would concatenate with a 'joining' element if supplied with a parameter, e.g.: def pets = ['ant', 'bee'] def food = ['bread', 'honey'] assert [pets, food].join() == ['ant', 'bee', 'bread', 'honey'] assert [pets, food].join('eats') == ['ant', 'bee', 'eats', 'bread', 'honey'] This conflicts somewhat with existing DGM join usage, so we are still thinking about our options here.
          Hide
          Ted Naleid added a comment - - edited

          I like it! But just to clarify for other readers, when you say "The first is for findResults which is basically findAll but automatically removing null values", I think you mean it's basically collect that removes nulls as collect returns the result of the closure, not findAll (which does filter nulls, but returns the item from the original request for truthy closure results).

          Is joinCollected exactly like findResults followed by a flatten()? Does it flatten all the way, or just one level deep? So if I return an array of arrays, does it preserve the nested arrays (unlike calling flatten())?

          Show
          Ted Naleid added a comment - - edited I like it! But just to clarify for other readers, when you say "The first is for findResults which is basically findAll but automatically removing null values", I think you mean it's basically collect that removes nulls as collect returns the result of the closure, not findAll (which does filter nulls, but returns the item from the original request for truthy closure results). Is joinCollected exactly like findResults followed by a flatten() ? Does it flatten all the way, or just one level deep? So if I return an array of arrays, does it preserve the nested arrays (unlike calling flatten() )?
          Hide
          Ted Naleid added a comment -

          Looked at the source and I see that it isn't the same as findResults + flatten() as it won't flatten all the way. I think this is desired behavior as we could always add a flatten() after

          I vote for naming it

          {joinCollected}

          over

          {sumCollected}

          , I think it's more descriptive, sum has other connotations for me.

          Show
          Ted Naleid added a comment - Looked at the source and I see that it isn't the same as findResults + flatten() as it won't flatten all the way. I think this is desired behavior as we could always add a flatten() after I vote for naming it {joinCollected} over {sumCollected} , I think it's more descriptive, sum has other connotations for me.
          Hide
          Paul King added a comment - - edited

          @Ted: Re: Clarification - my mistake, I meant collect - fixed in the original comment.

          Basically findResult is like collect then findNonNull (if Groovy had a findNonNull) though more efficient and findResults is like collect then findAllNonNull (assuming Groovy had a findAllNonNull).

          The main difference to joinCollected is that findResults is looking for nulls or not - joinCollected is always joining resulting collections.

          The downside to the "join" name proposal is that join is already taken in DGM for the producing Strings and we haven't worked out how to deal with that yet - so it still might end up being sumCollected.

          Show
          Paul King added a comment - - edited @Ted: Re: Clarification - my mistake, I meant collect - fixed in the original comment. Basically findResult is like collect then findNonNull (if Groovy had a findNonNull ) though more efficient and findResults is like collect then findAllNonNull (assuming Groovy had a findAllNonNull ). The main difference to joinCollected is that findResults is looking for nulls or not - joinCollected is always joining resulting collections. The downside to the "join" name proposal is that join is already taken in DGM for the producing Strings and we haven't worked out how to deal with that yet - so it still might end up being sumCollected .
          Hide
          Paul King added a comment -

          Fleshed out the patch to include map variants. Went back to using 'sumCollected' as the name for now until we can work out an alternative to 'join' - possibly 'adjoin', 'conjoin', 'concat'.

          Show
          Paul King added a comment - Fleshed out the patch to include map variants. Went back to using 'sumCollected' as the name for now until we can work out an alternative to 'join' - possibly 'adjoin', 'conjoin', 'concat'.
          Hide
          Paul King added a comment -

          Added findResults but didn't add joinCollected/sumCollected/bind/flatMap/mapCat. As the Closure version of sum goes someway to implementing this capability already:

          def nums = 1..5
          def squaresAndCubesOfOdds = nums.sum{ it % 2 ? [it**2, it**3] : [] }
          assert squaresAndCubesOfOdds == [1, 1, 9, 27, 25, 125]
          

          The proposals here would be using probably addAll() instead of allow overriding of "plus" as sum does but in any case I think further discussion is required. That can be the subject of a new jira if needed.

          Show
          Paul King added a comment - Added findResults but didn't add joinCollected/sumCollected/bind/flatMap/mapCat . As the Closure version of sum goes someway to implementing this capability already: def nums = 1..5 def squaresAndCubesOfOdds = nums.sum{ it % 2 ? [it**2, it**3] : [] } assert squaresAndCubesOfOdds == [1, 1, 9, 27, 25, 125] The proposals here would be using probably addAll() instead of allow overriding of "plus" as sum does but in any case I think further discussion is required. That can be the subject of a new jira if needed.

            People

            • Assignee:
              Paul King
              Reporter:
              Ted Naleid
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: