CODEWITHSUNDEEP
javaarraysloops
Adam
Adam

Reputation: 364

Using ArrayList instead of an array to avoid using a normal for loop

I'm reviewing a piece of code that looks like this:

public WrappedItem[] processItems(Item[] items)
{
    List<WrappedItem> wrappedItems = new ArrayList<>();   
    for (Item item : items)
    {
        /* …do something with the item… */
        wrappedItems.add(WrappedItem.wrap(item));
    }
    return wrappedItems.toArray(new WrappedItem[0]);
}

A list is created so that the enhanced for loop can be used and then it is converted to an array to be returned.

Would this be better from a performance and code style perspective? 

public WrappedItem[] processItems(Item[] items)
{
    WrappedItem[] wrappedItems = WrappedItem[items.length];   
    for (int i = 0; i < items.length; i++)
    {
        Item item = items[i]
        /* …do something with the item… */
        wrappedItems[i] = WrappedItem.wrap(item);
    }
    return wrappedItems;
}

Upvotes: 1

Views: 135

Answers (3)

Holger
Holger

Reputation: 298579

As your code is written in the question you are not wrapping the item (despite the name) but just putting into a differently type array. This can be done even easier:

public WrappedItem[] processItems(Item[] items)
{
  WrappedItem[] wrappedItems
    = Arrays.copyOf(items, items.length, WrappedItem[].class);
  for (Item item : items)
  {
    /* …do something with the item… */
  }
  return wrappedItems;
}

If you need Java 5 compatibility you can use

WrappedItem[] wrappedItems = Arrays.asList(items).toArray(new WrappedItem[0]);

instead of Arrays.copyOf.

Now that you have updated your question it’s clear that it’s not that easy. Then the answer is that the List based version creates some overhead but in most practical scenarios it will be negligible. As pointed out by others you can also combine a for-each loop with an additional index variable but to me it doesn’t look convincing as the index variable has the wrong scope (you have to declare it outside the loop) and it’s not really simpler than the traditional for loop.

Maybe the final words come from the Java 8 solution:

public WrappedItem[] processItems(Item[] items) {
  return Arrays.stream(items)
      .map(item -> {
          /* …do something with the item… */
          return WrappedItem.wrap(item);
      }).toArray(WrappedItem[]::new);
}

or

public WrappedItem[] processItems(Item[] items) {
  return Arrays.stream(items)
      .peek(item -> {
          /* …do something with the item… */
      }).map(WrappedItem::wrap).toArray(WrappedItem[]::new);
}

Upvotes: 0

katis
katis

Reputation: 21

From code style perspective using enhanced loop increases readability, as it's more abstract than "standard" for loop.

Standard for increases control, but introduces also a space for possible errors, as you're manipulating the indices and lengths, which everybody is familiar with, but still it's exposing some of the internal data's structure which is somehow against encapsulation...

Have a look also at this post What are the Advantages of Enhanced for loop and Iterator in Java?

Upvotes: 0

bowmore
bowmore

Reputation: 11308

There is no reason you cannot simply directly construct an array and make use of the advanced for loop :

public WrappedItem[] processItems(Item[] items)
{
    WrappedItem[] wrappedItems = WrappedItem[items.length];
    int i = 0;
    for (Item item : items)
    {
        /* …do something with the item… */
        wrappedItems[i++] = item;
    }
    return wrappedItems;
}

Upvotes: 2

Related Questions