3

After reading rules of Clean Code, one of them is to use default constructor when using transferring data from one collection to other.

For example i have :

Dictionary<string, string> dict = new Dictionary<string,string>();

So what is best for performance? I know that LINQ is not effective, but is always better way to use default constructor (for this specific situation).

1. List<string> list = new List(dict.Keys);

or

2. List<string> list = dict.Keys.ToList();
NSKBpro
  • 219
  • 1
  • 7
  • Possible duplicate of [Is micro-optimisation important when coding?](https://softwareengineering.stackexchange.com/questions/99445/is-micro-optimisation-important-when-coding) – gnat Jul 03 '18 at 07:48
  • And assuming you're interested in "clean" code: regardless this micro-optimization (already addressed in the answer) you should ask yourself: "Is it more clear?". When using ctor do you COPY the list or you just keep a reference? Is it true for all collections? Is it the expected behavior in ALL your classes? Is it limited to collections or is it valid to every other clsss which accepts a list as parameter? – Adriano Repetti Jul 03 '18 at 11:48
  • What makes you think there is any difference at all? This post is unrelated to software engineering, as is the book referenced. – Frank Hileman Jul 06 '18 at 01:13

1 Answers1

8

After reading rules of Clean Code, one of them is to use default constructor when using transferring data from one collection to other

A default constructor is one that either has no parameters, or where all of the parameters have default values. So:

var dict = new Dictionary<string,string>();

is using a default constructor. It is highly unlikely that any clean code advice would be suggesting you use such constructors for copying collections from one form to another.

I know that LINQ is not effective

Really? Again I think you are using the wrong word as linq is incredibly effective. It's not always the fastest approach though, so perhaps you mean "LINQ is not always the fastest"?

So what is best for performance?

Only you can determine this for your particular application. So grab yourself something like BenchmarkDotNet and benchmark the two approaches.

As a bit of a clue though, have a look at the source code and you'll see that:

var list = dict.Keys.ToList();

calls the following code:

public static List<TSource> ToList<TSource>(this IEnumerable<TSource> source)
{
    if (source == null)
    {
        throw Error.ArgumentNull(nameof(source));
    }

    return source is IIListProvider<TSource> listProvider 
        ? listProvider.ToList() 
        : new List<TSource>(source);
}

(code slightly reformatted to fit here better) ie, it simply calls new List<TSource>(source). So claims that linq is slow is a classic case of naively repeating an over-simplification rather than taking the time to check its validity first.

David Arno
  • 38,972
  • 9
  • 88
  • 121
  • Notably there is something (presumably) *even faster* than `new List(source)`, if the input is an `IIListProvider` (which is an *awful* name, at least it's `internal`). It looks to be related to slicing existing `List`s – Caleth Jul 03 '18 at 10:25