-1
\$\begingroup\$

I have a class that needs to provide access to an internal List<int> member. The functions that use this class need to be able to work with that list, mutating the contents and sorting/filtering the list etc, without changing the internal list in the class object. Having a method that provides a copy of the list instead of a getter seems like the logical solution.

I know that the following code works, but as this seems like a somewhat common design pattern I wanted to know if there was a better or more standard way to design the signature or body of this type of function to inform/enforce its use. Perhaps it should use ref or out? Would it be better to just return a copy instead of doing it via a parameter?

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

public void CopyReportsList(List<int> outList)
{
    outList = this.integerList.ToList();
}
\$\endgroup\$
1
\$\begingroup\$

Take it on its readability deference :

Example 1 (Current Usage):

CopyReportsList(outList);

Example 2 (Common):

var outList = GetIntegerList();

Example 3 (Common):

CopyReportsList(source, out outList);

if you compare them, you'll see that anyone would read example 1 at first would think it's meant to copy outList into another collection, which means, there is some other process going. While example 2 and 3 would be read as having an identical copy of current stored list.

So, no one would ever use example 1, because it's unclear, confusing, and not a good practice.

Example 2 would be clearer, and it's most used.

Example 3 would be also used in some cases, but mostly, its most known uses for TryParse and Copy functionalities.

For your case, example 2 would be fine, but you would need to use a Property instead.

public IEnumerable<int> Integers 
{
    get { return integerList; }
}

usage :

var data = new SomeClass();

var list = data.Integers;

But you have to name your property to something that would be related to your logic functionality. Also, for returning type, if the return type is type of collection or array, try to use IEnumerable interface it would be better. (same as I did in the property example). The reason is that IEnumerable interface is implemented on all collections types including arrays. So, this would give your implementation more flexibility by providing the minimal understanding of the returning type which would be easier to extend to other collections types for current and long term running.

another note on this line :

outList = this.integerList.ToList();

integerList is a List<int>, so there is no need to use ToList(), what are you doing is converting a list to a list ! which is redundant and unnecessary.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks for the input. The reason i was calling ToList() is so that it makes a copy of the list instead of returning a references to the list. I don't want the internal list to be changed by external function, but those function need to be able to freely work with their own "copy" of the list they are given. \$\endgroup\$ – user3776749 Feb 27 at 21:35
  • 1
    \$\begingroup\$ @user3776749 it won't change the source for the current context. It'll return a copy of the source. So, if you do this return integerList; or var outList = this.integerList; it'll make an identical copy, any changes on outList won't be applied on integerList. \$\endgroup\$ – iSR5 Feb 27 at 21:41
  • \$\begingroup\$ @iSR5 that is not true at all, lists are reference types, returning wont make any copy you need to call ToList() or reinitialize the list with new List<>(IEnumerable<>) if you want to avoid this behavior. Furthermore, if your values were also reference type you would need to make a deep copy, to avoid any external changes being reflected on the collection. But for this case the aforementioned suggestions will do the job. \$\endgroup\$ – Denis Mar 2 at 13:52

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