current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
stack overflow careers
Sign up ×
Stack Overflow is a community of 4.7 million programmers, just like you, helping each other. Join them; it only takes a minute:
up vote 1 down vote favorite

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;
}
share|improve this question

migrated from codereview.stackexchange.com Jul 30 '14 at 15:10

This question came from our site for peer programmer code reviews.
2  
Perhaps you fail to realize that the enhanced for loop works just fine on arrays too. – bowmore Jul 30 '14 at 15:09
    
Nope, he did notice, but I am more concerned that this is example code (since item is not wrapped when put in wrappedItems). This is for Stack Overflow. – rolfl Jul 30 '14 at 15:10
5  
Why is this migrated from code review? – Mena Jul 30 '14 at 15:11
    
Because code review deals with real code, that works, and this code, does not. – rolfl Jul 30 '14 at 15:12
    
@bowmore No I noticed. But the author preferred the enhanced for loop. – Adam Jul 30 '14 at 15:13

3 Answers 3

up vote 2 down vote accepted

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;
}
share|improve this answer
1  
Same answer, same time, I'll drop mine. – rolfl Jul 30 '14 at 15:25
up vote 0 down vote

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);
}
share|improve this answer
up vote 0 down vote

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?

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.