CodeSOD: A Simple List Copy

Mike's team had a new hire. They did great on the interview, really shined in the coding challenges, so it was a no-brainer hire. In the middle of on-boarding, the team got slammed, so this new hire ended up being left to fend for themselves.

This was a mistake.

public class ElementHandler {private final List<Element> elements = new ArrayList<Element>();public final List<Element> getElements() {final Element[] allElements= elements.toArray(new Element[elements.size()]);final List<Element> result = new ArrayList<Element>();for(int i =0; i<allElements.length; i++){result.add(allElements[i]);}return result;}}

The getElements function here looks like a basic accessor method. Clearly, the developer was concerned about just returning a reference to the class's internal state- a wise concern- and opted to return a copy.

They then just failed to use any of the basic .NET Framework methods for making a copy and made their own. Badly.

First, they convert the list toArray. This does a deep copy on all the elements. Then they iterate across all of those elements to populate a new List, which is the return value.

There are some fun .NET internals here- allocating an empty ArrayList creates an array list with an internal buffer of 0 elements. When you add the first element, the ArrayList creates a buffer that can hold 4 elements. When that buffer gets filled, the ArrayList creates a new buffer, twice that size, and copies all the elements into the new buffer. Each time the buffer fills up, all the items get copied again.

So, depending on how big this input list is, this code could be inserting a lot of copy operations that you never see, copying the same elements over and over again.

What makes this so frustrating is that .NET has a perfectly simple way to create a copy and preallocate an array of the correct size, all in one step:

return new ArrayList<int>(elements)

This version has only one copy operation per element in the list, not potentially many. And, honestly, it's easier to read and clearer about its purpose.

But, I guess the "right" way doesn't give you the option to stress test the garbage collector.

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!