[jira] [Commented] (COLLECTIONS-573) Add CollectionUtils#deepMerge method

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/COLLECTIONS-573?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16521456#comment-16521456 ] 

ASF GitHub Bot commented on COLLECTIONS-573:
--------------------------------------------

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

    https://github.com/apache/commons-collections/pull/12#discussion_r197633792
  
    --- Diff: src/main/java/org/apache/commons/collections4/CollectionUtils.java ---
    @@ -1889,4 +1889,66 @@ public static int maxSize(final Collection<? extends Object> coll) {
             }
             return collection.iterator().next();
         }
    +    
    +    /**
    +     * Method recursively finds deepest content of nested iterables and 
    +     * merge them into one chosen {@link Collection}. Method accepts 
    +     * {@link Iterable} argument only if it has at least one level of nesting.
    +     * {@code Collection} argument must bound deepest elements type. Because in Java
    +     * don't exist any good and convenient way to check bound type there is no way
    +     * to prevent inserting bad values to wrong bounded types. It's possible to create
    +     * list with non valid bounded type. It will result ClassCastException throw at runtime
    +     * if {@code collection} won't be cast to proper type.      
    +     * <p>
    +     * Current implementation have time complexity {@literal O(n^k)} 
    +     * where {@literal k} is a level of iterables (1 = no nested).
    +     * </p><p>
    +     * <b>Example:</b><br>
    +     * <code>{@literal List<String> list = 
    +     * CollectionUtils.mergeDeep(Set<Set<Set<Set<String>>>> setOfSets, new ArrayList<String>)}</code>
    +     * </p>
    +     * 	If one banch of set contains ("foo","bar"), and second one contains 
    +     * ("faz" "foz") than after method use {@code list} instance contains ("foo", "bar", "faz", "foz").
    +     * @param <E> deepest element type of iterableOfiterables parameter
    +     * @param <T> {@code collection} with bounded {@literal <E>} 
    +     * @param iterableOfIterables an {@code object} which implements {@code Iterable} interface and has
    +     *  nested another {@code iterable} object 
    +     * @param collectionToFill an {@code collection} instance to fill up by deepest {@code iterable} content  
    +     * @return {@code collectionToFIll} parameter filled up by deep content {@code iterableOfIterables} parameter
    +     * @throws NullPointerException when one of a parameters is null
    +     * @throws ClassCastException at runtime if {@code collectionToFill} is not bounded with valid parameter
    +     * @since 4.1 
    +     */
    +	public static <T extends Collection<E>,E> T deepMerge(final Iterable<? extends Iterable<?>> iterableOfIterables, 
    +			final T collectionToFill) {
    +		Iterator<? extends Iterable<?>> iterator = iterableOfIterables.iterator();
    +		if (!iterator.hasNext()) {
    +			return collectionToFill;
    +		}
    +		while (iterator.hasNext()) {
    +			deepMergeRecursion(iterator.next(), collectionToFill);
    +		}
    +		return collectionToFill;
    +	}
    +	
    +	@SuppressWarnings("unchecked")
    +	private static <T extends Collection<E>,E> void deepMergeRecursion(final Iterable<?> iterable,
    +			final T collectionToFill ) {
    +		Iterator<?> iterator = iterable.iterator();
    +		if (!iterator.hasNext()) {
    +			return;
    +		}
    +		Object firstElement = iterator.next();
    +		if (!(firstElement instanceof Iterable<?>)) {
    +			collectionToFill.add((E)firstElement);
    +			while (iterator.hasNext()) {
    +				collectionToFill.add((E)iterator.next());
    +			}
    +			return;
    +		}
    +		deepMergeRecursion((Iterable<?>) firstElement, collectionToFill);
    +		while (iterator.hasNext()) {
    +			deepMergeRecursion((Iterable<?>) iterator.next(), collectionToFill);
    --- End diff --
    
    I suggest to change the order here:
    1. change the condition `if (!(firstElement instanceof Iterable<?>)) {` -> `if (firstElement instanceof Iterable<?>) {`
    2. add the `else `statement
    3. remove `return;`
    
    this will simplify the code as there will be:
    * more clear condition
    * no return statement in the middle of the method
    
    



> Add CollectionUtils#deepMerge method
> ------------------------------------
>
>                 Key: COLLECTIONS-573
>                 URL: https://issues.apache.org/jira/browse/COLLECTIONS-573
>             Project: Commons Collections
>          Issue Type: New Feature
>            Reporter: Bertrand Wlodarczyk
>            Priority: Major
>
> Placeholder ticket for pull request: https://github.com/apache/commons-collections/pull/12



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)