CODEWITHSUNDEEP
c#.netgenericsproperties
demonplus
demonplus

Reputation: 5801

AddRange in give public access to list

I have some legacy C# code with the following piece:

private List<MyClass> mList = new List<MyClass>();

public List<MyClass> getList()
{
   List<MyClass> list = new List<MyClass>();
   list.AddRange(mList);
   return list;
}

Not sure what is the purpose of AddRange here? Can I rewrite it as:

public List<MyClass> getList()
{
   return mList;
}

Upvotes: 2

Views: 299

Answers (5)

Arghya C
Arghya C

Reputation: 10078

Tim's comment and Sloth's already gives correct explanation.

Just an additional point:

private List<MyClass> mList = new List<MyClass>();

public List<MyClass> getList1()
{
    List<MyClass> list = new List<MyClass>();
    list.AddRange(mList);
    return list;
}

public List<MyClass> getList2()
{
    return mList;
}

getlis1() -
(1) Keeps the original list safe. Add() or AddRange() or Clear() doesn't affect it.
(2) Throws exception if mList is null!

getlis2() -
(1) Exposes the original list mList. Add() or AddRange() or Clear() affects (modifies) it.
(2) Returns null if mList is null!

Upvotes: 0

sloth
sloth

Reputation: 101122

No.

If you simply use return mlist, you return the mlist instance, whereas the original code returns a shallow copy of it.

Say your class looks like this:

class Foo
{
    public Foo()
    {
        mList.Add(1);
    }

    private List<int> mList = new List<int>();

    public List<int> getList()
    {
        List<int> list = new List<int>();
        list.AddRange(mList);
        return list;
    }
}

And now you run

var x = new Foo();
x.getList().Add(2);
x.getList().Add(3);

The content of mList will still be a single 1, because the calls to getList returned copies of mList, not the list itself.

If you change the code like you did in your question, you would have altered mList it would now contain the elements 1, 2, and 3.

It's not clear from the method name that a copy is returned, so you maybe want to change it to something like GetListCopy (in which case the method could simply return new List<MyClass>(mList) or mList.ToList()), or return the list as a IReadOnlyList to make clear that the list should not be changed.

public IReadOnlyCollection<MyClass> getList()
{
    return mList.AsReadOnly();
}

Upvotes: 5

Dennis
Dennis

Reputation: 37780

The only purpose here is to clone original list to keep it unchanged. But, it is better to re-write this property and turn it to the method:

public List<MyClass> getList()
{
   // note, that Enumerable.ToList() does the same
   return new List(mList);
}

List<T> constructor checks for ICollection<T> implementation in source IEnumerable<T>, and properly sets initial capacity. Since every call will create new instance, this shouldn't be a property, but a method.

Upvotes: 0

Fabjan
Fabjan

Reputation: 13676

As already pointed if you just use return mList; it'll return the original list and not a copy of it.

However you can simplify it by using :

public List<MyClass> getList
{
   return mList.ToList();  // returns copy of original list
}

Upvotes: 2

user3596113
user3596113

Reputation: 878

No you cannot. This might result in some unexpected behaviour in you software because the code as it is creates a new instance of List and your solution has the same instance returned everytime the method is called.

Upvotes: 0

Related Questions