groovy

See GROOVY-2903 ( http://jira.codehaus.org/browse/GROOVY-2903 ) for more on this issue.

Alternative patch

I'd suggest that flatten-with-closure get moved into its own ticket – it's an interesting idea, but really tangential to issue being considered here. #flatten specifies that it recursively adds Collections, which is the most straightforward and easily conceptualized approach, so that's what my patch did. We should at least fix the code to adhere to its contract, and then we can discuss flatten-with-closure syntax and behavior.

The problem with recursively adding map content (or the superset of that functionality, recursively adding any iterable thing in the list), is that it's really going to wreak havoc on unit tests. If you "flatten" maps in some way, this means that you're getting different behavior if you use a bean or a hash map in its place: this is exceedingly common in unit testing, including making it onto the website as a recommended pattern.

Any patch which pushes the "flatten maps" approach also needs to include a patch to create a map#flatten which has parallel behavior.

When you say "which has parallel behavior", what did you mean exactly?

If a list containing a map somehow mangles that map, then map#flatten should mangle the map in the same way. This will retain the "recursive" flavor of #flatten.

altered patch for Collection supporting arrays; defaults to no flattening of maps

This patch isn't quite correct. Whatever you're going to do to the children needs to be symmetric wrt to what you do at the top-level.

If you're going to flatten arrays, then (Object[]).flatten() needs to work too. If you're gonna do that you have to implement DGM.flatten(Object) and DGM.flatten(Object, Closure).

yes, or else int[][][] for example could not be "flattened".. which makes me think, that we maybe should not flatten arrays

What's wrong with flattening arrays?

nothing, it was just a thought. If we allow flatten for any array, then a DGM method won't do it. I then suggest that we handle it like we do the size() method on arrays.. by special code from inside the MetaClass. Of course then flatten won't show up in the API

Patch for Collection applied. I am still not sure I see the value in doing this for arrays.

Can someone who wants array flattening please provide some use cases, otherwise I think I will close this issue. The current patch would provide value in that you could convert an array to a Collection then flatten it then convert back.

Implemented for Collections. Please reopen if you have some use cases not covered.

I really can't see any justification for flattening arrays which are elements of a collection but not allowing arrays to be flattened. It should be one way or the other.

This:

[[[1, 2, 3, [1, 2, 3, [1, 2, 3] as Object[]] as Object[]] as Object[]] as Object[]].flatten()
==>
[1, 2, 3, 1, 2, 3, 1, 2, 3]

is far too inconsistent with this:

([1, 2, 3] as Integer[]).flatten()
==>
groovy.lang.MissingMethodException: No signature of method: [Ljava.lang.Integer;.flatten() is applicable for argument types: () values: {}

A significant case on why that is important is that a user wondering whether arrays are flattened or not will try the simple (second) case first and conclude that arrays are not flattened. They will be greatly and not pleasantly surprised when they discover that is just a special case and that Groovy does flatten arrays, but only when the top-level is a Collection.

It is an interesting edge case. What would you expected {{([1, 2, 3] as Integer[]).flatten()}} to return? Just the original elements but now in a List? Normally flatten() is trying to preserve the original type. If we preserve the original type, we could just use the unity function except for Object[] which would require further processing and perhaps multi-dimensional arrays going to single (if we aren't expecting the multi-dimensional nature of the original array to be preserved).

I wouldn't expect any different btw {{([1, 2, 3] as Integer[]).flatten()}} and {{([1, 2, 3] as Object[]).flatten()}}, just as there is no difference btw {{([[1, 2, 3] as Integer[]]).flatten()}} and {{([[1, 2, 3] as Object[]]).flatten()}}.

So yes, the result is a List, which is what createSimilarOrDefaultCollection creates.

And flatten as it is currently implemented is flattening multidimensional arrays (as my sample showed), so I don't understand what you're saying there.

Hmmm, I suppose this createSimilar jazz for arrays could be extended to consider element type for generics, but that goes beyond this issue (and there is Collection.flatten(Collection) which could be used).

I do agree that either arrays should be flattened or they shouldn't – pick one. The inconsistency is confusing. I vote for not flattening them, on the basis that they aren't collections. But I really don't care, since I always "as List" my arrays as soon as they get dumped into Groovy.

Unless there's been something awesome that I've missed out on, generic code is type-erased in Java, which means that the run-time has no idea what the generic type originally was, and there's no value in trying to discern or enforce it in "createSimilarOrDefaultCollection".

I wouldn't expect any different btw ([1, 2, 3] as Integer[]).flatten() and ([1, 2, 3] as Object[]).flatten(), just as there is no difference btw ([[1, 2, 3] as Integer[]]).flatten() and ([[1, 2, 3] as Object[]]).flatten(). So yes, the result is a List, which is what createSimilarOrDefaultCollection creates.

Your interpretation is exactly the one I would expect; but you like I know about createSimilarOrDefaultCollection. I am just playing devil's advocate and wondering what our users think. Perhaps we just haven't defined createSimilarOrDefaultCollection fully yet to take into account arrays. If we say flatten is type preserving, why wouldn't I expect it to return an array? I think we can argue either way and I think we will end up with your interpretation, just not wanting to jump the gun.

And flatten as it is currently implemented is flattening multidimensional arrays (as my sample showed), so I don't understand what you're saying there.

Yes, but I can also imagine it would be useful to have for example a flatten which took int[][][] and returned int[].

I'm not talking about Groovy's whole collection type system wrt arrays. I'm saying the current implementation of flatten is bugged and it needs some sort of fix.

That said, I'm fairly sure that the only sensible interpretation is that Groovy coerces arrays to collections. To suppose that flatten might return an array rather than a collection also means that you would have functions like collect, find, and grep return an array rather than a collection when performed on an array. That wouldn't be a very good approach because any actual implementation would have to use variable sized collections and then convert to an array at the end, which is exactly the thing that the user will do if that is what he wants.

@Jim, yes very thoughtful comments. I think you have all the thinking spot on. Apologies for erring on the side of socializing such thinking behind the language in this forum. Until we have that Wings language spec in place it can't hurt to discuss things a bit ... well, not hurt too much!?

To reiterate your last comment and reinforce the need for further work on this issue, if findAll already converts arrays to lists as shown below, why not flatten:

def nums = 1..10 as int[]
assert nums instanceof int[]
def evens = nums.findAll{ it % 2 == 0 }
assert evens == [2, 4, 6, 8, 10]
assert evens instanceof List

I think that (along with earlier examples) seems pretty convincing to me.

If arrays are going to be lists, they should implement the full list API. Having arrays be half-lists from the view of duck typing just leads to confusion, bugs, and cognitive dissonance.

No worries, Paul. Some of the RC issues and a lack of sleep and other such stuff have made be a bit cranky.

Glad to see that we have arrived at consensus.

Amended to also handle arrays as base collection type