To new() or to Clear(), that is the question.

Question

At my current workplace, I run across a lot of code that looks similar to the following example:

private void GetWidgets(ref List<Widget> widgets){
  if(widgets == null){
    widgets = new List<Widget>();
  }
  else{
    widgets.Clear();
  }
  ... code to fill widget list
}

and

public class Widgets{
  private List <Widget> widgets;
  ... other private members

  public Widgets(){    
    Clear();
  }

  public void Clear(){
    if(widgets == null){
      widgets = new List<Widget>();
    }
    else{
      widgets.Clear();
    }
    ... initialize other private members
  }
}

I personally find the usage of the Clear method in these examples to make the code uglier and more complicated. I don't know if there is a performance increase in using the Clear method over simply creating a new List, but I would prefer code that look like this:

private List<Widget> GetWidgets(){
  widgets = new List<Widget>();
  ... code to fill widget list;
  return widgets;
}

and

public class Widgets{
  private readonly List<Widgets> widgets = new List<Widget>();
  ... other private members

  public Widgets(){        
    ... initialize other private members
  }

  public Clear(){
    widgets.Clear();
  }
}

Code like this forces you to change your development patterns slightly, but I think it makes the code far more readable and reduces the complexity.

Aside from the fact that this is a piece of hastily written sample code, I rampantly used concrete classes instead of interfaces, etc.; What are your opinions? What are the pros and cons of each?

Answer 1

Clear is a better option if you have a shared reference of your list that other classes can access. In that case, newing it up will create a new empty list, but any other classes that have created references to the old list will still see a filled list.

Imagine this scenario:

List<Widget> first;
myWidgets.GetWidgets(ref first);
List <Widget> second = first;
myWidgets.GetWidgets(ref second);

In the above case, if Clear was employed, first and second would be connected to the same list. If new was employed, they would be connected to two completely different lists. There are consequences with either approach, and the decision to use one over the other would depend on what you are trying to accomplish.

So I guess the answer to your question is:

• new is better when you want to ensure that each call to GetWidgets creates an independent, essentially immutable list. This is better in multithreaded applications. You're even better served if you return the list as a readonly list.
• Clear is better when you want to have a single list represented, and changes to that list are seen by all classes referencing it. This creates many contention issues if you're multithreading.

Answer 2

If there are references to this list that would require this type of code then I think something needs to be redesigned. Try for side-effect free functions, which this is not since it is both constructing objects and manipulating state which may have consequences in other places.

Answer 3

Personally, I like

public class Widgets {
  private readonly List <Widget> widgets = new List<Widget>();
  ... 
  public void Clear() {
      widgets.Clear();
    ...
  }
}

The list is created in one place and one place only; Clear() is unambiguous, straightforward, uncomplicated, and unconditional. Less code, less complexity.

I'm using readonly here to remind me - forcibly, when I write wrong code - that I'm done creating the list, that it would be a mistake to re-create it.

Updated with correct C# syntax thanks to Joel Mueller's helpful comment.

Answer 4

There is also garbage collection to consider - if you are using new() a lot, then that will create lots of objects on the heap that will have to be garbage collected, but Clear() won't create any new heap objects (although this is only really an issue if you're doing this thousands of times)