I have this small utility for selecting from a collection via multiple indices at one method invocation:

MultiIndexUtils.java

package net.coderodde.util;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;

public final class MultiIndexUtils 

 private MultiIndexUtils() 

 public static <T> List<T> multiIndex(Collection<T> collection,
 Collection<Integer> indices) 
 List<T> indexedElements = new ArrayList<>(indices.size());
 T array = (T) collection.toArray();
 indices.forEach((Integer index) -> 
 indexedElements.add(array[index]);
 );
 return indexedElements;
 

 public static <T> List<T> multiIndex(Collection<T> collection,
 Integer... indices) 
 return multiIndex(collection, Arrays.asList(indices));
 

 public static void main(String args) 
 List<String> strings = Arrays.asList("A", "B", "C", "D", "E", "F");
 System.out.println(multiIndex(strings, 1, 0, 3, 2, 0, 5, 4));
 

Critique request

I would like to hear anything that comes to mind.

First up, nice touch with BADCAFE :)

Then, for my two cents:

• To me it feels a bit strange using Collection for the element supplier type. Since we are indexing values, it would seem strange to use it on a Set for example, as it does not impose order.


• On a similar note, using a Collection for the indices parameter seems strange as well, for the same reasons. A Set also implies uniqueness, so no two equal indices would be passed.

Both would seem to make more sense being Lists to me. But this is something I am just saying based on what I am feeling :)

Continuing:

• 
  

  You can relax the type bound on the input parameter of the elements supplier, like so:

  
  
  

  public static <T> List<T> multiIndex(final Collection<? extends T> collection, ...
  

  
  
  

  Now you can pass a collection parameterized with T or with a subtype of T, which makes it more versatile.

  
  


• 
  

  Seeing as you are extracting elements, you can annotate the array cast with a SuppressWarnings, as it is safe:

  
  
  

  @SuppressWarnings("unchecked")
  final T array = (T) collection.toArray();
  

  
  


• 
  

  Finally, I'd generally think twice before using forEach on a Stream, as it implies using stateful operations. Personally, I think in this case it can be more cleanely written as:

  
  
  

  @SuppressWarnings("unchecked")
  T array = (T) collection.toArray();
  return indices.stream()
   .map(index -> array[index])
   .collect(Collectors.toList());
  

  
  

Just a note to keep in mind:

There is no rule that utility classes (in the sense that they provide common behavior) have to be staticand not insatiable. Thats just a misconception because classes with only static methods used to be called utility classes (in the sense that they have only static methods).

Static access to methods and class members makes the code using it tight coupled and therefore hard to reuse, extend or otherwise maintained. It effectively blocks dependency injection and polymorphism, two of the major reasons why we use an object oriented language in the first place.

So when creating your classes you should not default to uninstantiable static classes.

I think the most important point was already mentioned: The elements should be contained in a List. The difference between a Collection and a List is exactly that: The List allows indexed access. And for a utility function that offers a "special form" of indexed access, it is reasonable that the function requires a List.

Think of it that way: When you call it with a List, you still have do the time- and memory-consuming toArray call, even though you already could use indexed access. You only "forgot" that, because you know the List only as a Collection.

More generally, you should think about what you internally need in order to properly do your work. This is the "core" method that you can implement. And in your case, you need something that you can access using indices, and the indices themself.

When you have this "core" method, you can still wrap convenience methods around it. So for example, if this is really crucial, you could even consider offering two versions of the method:

public static <T> List<T> multiIndex(Collection<T> collection, ...) 
 return multiIndex(new ArrayList<T>(collection), ...);

public static <T> List<T> multiIndex(List<T> list, ...) 
 return multiIndex(list, ...);

When you call it with a List, it will call the List version. For other collections, it will call the Collection version that converts the collection into a List. In any case, you can internally work with what you need: A List, which provides the necessary indexed access, that you otherwise had to quirkily allow with the toArray call.

The indices raise similar questions. If the method is supposed to be called like you showed it in the example, you could use int instead of Integer. If using a Collection<Integer> is a case that you explicitly want to cover, you have several options of how to implement your "core" method:

<T> List<T> multiIndex(List<? extends T> list, int ... indices)
<T> List<T> multiIndex(List<? extends T> list, Collection<Integer> indices)
<T> List<T> multiIndex(List<? extends T> list, IntStream indices)
<T> List<T> multiIndex(List<? extends T> list, Stream<Integer> indices)

Again, you can create wrappers in the one or the other direction. For example, you can convert the int ...indices array into a Stream<Integer> by calling

IntStream.of(indices).boxed()

or vice versa, convert the Collection<Integer> into an IntStream by calling

indices.stream().mapToInt(Integer::intValue)

In any case, note that the "core" method can probably be fairly trivial in the end:

public static <T> List<T> multiIndex(List<? extends T> list, IntStream indices) 
 return indices.mapToObj(list::get).collect(Collectors.toList());

Another hint: Depending on your exact use cases, you could also consider to just create a view on the required elements of the list:

private static <T> List<T> multiIndexView(
 List<? extends T> list, int ... indices)

 return new AbstractList<T>()
 
 @Override
 public T get(int index)
 
 return list.get(indices[index]);
 

 @Override
 public int size()
 
 return indices.length;
 
 ;

Changes in the original list will then be visible in this view. (If you also implement the set method, also vice versa). So then you can do the following:

public static void main(String args) 
 List<String> strings = Arrays.asList("A", "B", "C", "D", "E", "F");

 List<String> view = multiIndexView(strings, 1, 0, 3, 2, 0, 5, 4);

 // Prints BADCAFE
 System.out.println(view);

 strings.set(2, "B");
 strings.set(5, "B");

 // Prints BADBABE :-)
 System.out.println(view);

If you still want a method that returns a new list, then you can, once more, simply wrap the call:

public static <T> List<T> multiIndex(List<? extends T> list, int ... indices) 
 return new ArrayList<T>(multiIndexView(list, indices));