Add a take method to Collections, Iterators, Arrays

There should also be a drop( n )

def a = [ 1, 2, 3 ]

assert a.drop( 0 ) == [ 1, 2, 3 ]
assert a.drop( 1 ) == [ 2, 3 ]
assert a.drop( 4 ) == []

Had a few minutes to come up with the implementation and tests for take with Arrays, Lists, and Iterators

Requires JavaDoc, and a look over to make sure it's not horribly wrong.

Added drop with tests for List, Array, and Iterator by closely following the take implementation.
Also, added a bit of Javadoc to both take and drop.
Replaced Arrays.copyOf() with ugly System.arraycopy() because copyOf is the new addition to JDK 1.6.

Took the liberty of moving take couple of methods down, because they were sitting between head and it's counterpart tail.

I very much advice to change the proposed method names.

take and drop are used for lazy sequences as in http://groovyconsole.appspot.com/script/459001 .

Accessing the first or last n entries could be named

first n

last n

which would align with the respective JDK methods.

This feature needs more discussion in the dev group
with regard to consistency of the concept and API.

Is it worth continuing the effort (following the take/drop nomenclature), and a function rename can occur when a decision is reached on the dev list?

I'll ask this on the dev list as well, as I am not sure anyone will get updates from this comment (no watchers on this issue)

Asked a question on the groovy-dev mailing list: http://groovy.329449.n5.nabble.com/Naming-for-take-drop-methods-GROOVY-4865-td4461123.html

Deleted my patch, and now using Dinko's to progress with adding the other cases and tests

Third pass (includes patch "second_take..." from Dinko Srkoc)

This is now part of the fourth pass attachment (see below)

Added Dinko Srkoc as an author.

Added take( n ) methods for CharSequence, InputStream and Reader along with initial tests.

Added since 1.8.2 to all new methods

Not sure whether the CharBuffer method for extracting from the Reader is overkill... Feels better than reading it a char at a time, but adds a dependancy on JDK having java.nio.* (I beleive this should be ok, as it has been there since 1.4)

Code review would be really helpful as always

(cross-posted from the dev list)

Been thinking about this, and I think I now agree with Dierk

For CharSequence, Array, List and Map, the function names take and drop don't seem right, as the original object is not (and shouldn't be) mutated, so

def a = 'tim'
println a.take( 1 )

will print t and the variable a will still be set to 'tim'

in these situations, I believe the methods should be called first and last

However, for Iterator, Reader and InputStream where the state of the object is changed by reading elements, I believe that take fits, so

def a = new StringReader('tim')
println a.take(1)

will print t, and the variable a will now be the Reader containing the remaining String 'im'

drop would (in these three cases) exhaust the input, so there would be nothing left in the Iterator, Reader or InputStream if it were interrogated again.

So in summary, my suggestion is:

CharSequence, Array, List and Map : first(n) and last(n)

and for:

Iterator, Reader and InputStream : take(n) and drop(n)

New attachment fourth_pass_drop_for_CharSequence.diff

This includes the drop method for CharSequence, and the tests for it.

Not sure what the best way to implement drop for Reader or InputStream is...

Agree with Dierk that take and drop should imply lazy. I don't think laziness is particularly useful for the common case of truncating a string, but here's a link to some code for treating Iterators/Iterables as lazy sequences, include take and drop methods: https://github.com/ataylor284/iteratorutils . I'd go with the names Dierk suggests, and save take/drop for something like that.

Removed my patch since Tim's fourth is the base for further progress.

New patch fifth_iteration.diff

Added drop and take for Map. Tried explaining in the javadocs that usage with a non-ordered Map could cause fun

Added some more documentation to some of the methods...

Think all that's left is drop for Reader and InputStream

Added sixth iteration patch with drop implemented for Reader and InputStream, with tests.

Other changes:

• fixed bug with negative parameter value in drop for list and array
• replaced Arrays.copyOf with System.arraycopy in take for InputStream since copyOf is added in JDK 1.6

Do you think Iterator.drop and Iterator.take should return Iterator<T>?

If so, do you feel Reader.drop/take and InputStream.drop/take should respect the same contract and return Readers and InputStreams?

PS: Nice catch with the Arrays.copyOf I completely missed that...

Added sixth_iteration_returning_iterators.diff

Basically the same as Dinko's sixth_iteration.diff but Iterator.take and Iterator.drop return Iterator<T> rather than List<T>

Unit tests have been updated accordingly.

Yes, I believe Iterator<T> should be returned. As a general rule, Foo.take/drop should return Foo. That would probably go for List.take/drop too, e.g. LinkedList.take/drop should return LinkedList.

I think that it would be slightly surprising if Reader/InputStream didn't return Readers and InputStreams. I must say though that I'm slightly uncomfortable with Reader/InputStream since, as Peter Mondlock said(1), they aren't really sequences or collections.

PS: thanks

(1): http://groovy.329449.n5.nabble.com/Naming-for-take-drop-methods-GROOVY-4865-tp4461123p4469521.html

Cool, so here is seventh_iteration.diff which has:

• Iterator.take and Iterator.drop returning Iterator<T>
• List.take and List.drop now call createSimilarList instead of new ArrayList<T> so if LinkedList.take is called, the returned List should be a LinkedList

I have absolutely no worries about removing the code for Reader and InputStream, as these seem to be a sticking point for this feature (and one of the causes of the argument over function names)

Removed sixth_iteration.diff.

I'm OK with removing Reader and InputStream code too, in the interest of more consistent take/drop.

Added eighth_iteration.diff (and removed seventh_iteration.diff

In this version, I have:

• Removed drop and take for Reader and InputStream
• Added comment about Hashtable.drop to the Map.drop method javadoc (unpredicatble results)
• Changed unit tests to test multiple types of List, Map and CharSequence
• Added a test for Symmetry (where the JDK allows it), so it ensures that Iterator.drop returns an Iterator, LinkedList.drop returns a LinkedList, etc
• Added groovyTestCase blocks to each method in the javadoc. Examples in the docs are worth 1000 words

Hope it's all ok... I'll post about this update to the dev list, to see if it is more acceptable to the group

Removed import java.nio.CharBuffer as it isn't needed with the removal of Reader

Fantastic! Thanks Guillaume! Looks like my @since 1.8.2 was too pessemistic! :-D

Tests failed on Java 5...

The reason is that javax.swing.text.Segment did not implement CharSequence in Java 5, but does in Java 6...

Here's a patch script to remove that class from the tests:

Index: src/test/groovy/GroovyMethodsTest.groovy
===================================================================
--- src/test/groovy/GroovyMethodsTest.groovy	(revision 22354)
+++ src/test/groovy/GroovyMethodsTest.groovy	(working copy)
@@ -947,7 +947,6 @@
         def data = [ 'groovy',      // String
                      "${'groovy'}", // GString
                      java.nio.CharBuffer.wrap( 'groovy' ),
-                     new javax.swing.text.Segment( 'groovy' as char[], 0, 6 ),
                      new StringBuffer( 'groovy' ),
                      new StringBuilder( 'groovy' ) ]
         data.each {
@@ -1015,7 +1014,6 @@
         def data = [ 'groovy',      // String
                      "${'groovy'}", // GString
                      java.nio.CharBuffer.wrap( 'groovy' ),
-                     new javax.swing.text.Segment( 'groovy' as char[], 0, 6 ),
                      new StringBuffer( 'groovy' ),
                      new StringBuilder( 'groovy' ) ]
         data.each {
@@ -1048,7 +1046,6 @@
           // CharSequences
           (java.lang.String)        : new String( 'groovy' ),
           (java.nio.CharBuffer)     : java.nio.CharBuffer.wrap( 'groovy' ),
-          (javax.swing.text.Segment): new javax.swing.text.Segment( 'groovy' as char[], 0, 6 ),
         ]
         data.each { Class clazz, object ->
             assert clazz.isInstance( object.take( 5 ) )

Sorry about that

Thanks from me too, Guillaume!

Shouldn't "take" also be documented in Groovy String class? Currently it isn't:

http://groovy.codehaus.org/groovy-jdk/java/lang/String.html

Ahhh, because take operates on the class CharSequence (which String implements), the documentation is there instead: http://groovy.codehaus.org/groovy-jdk/java/lang/CharSequence.html