Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Possible optimization #135

Closed
nsimeonov opened this Issue · 3 comments

2 participants

@nsimeonov

A friend showed me Massive and I was absolutely amazed by your little gem. So I was trying to figure out how it works and found this code:

nv.Cast().Select(key => new KeyValuePair(key, nv[key])).ToList().ForEach(i => d.Add(i));

shouldn't you use AllKeys instead of Cast - like this:

nv.AllKeys.Select(key => new KeyValuePair(key, nv[key])).ToList().ForEach(i => d.Add(i));

With AllKeys the code looks somewhat clearer.

Regards and thanks for sharing your excellent code!

@robconery
Collaborator

Thank you for the good thoughts :). A couple of thoughts for you: I love that you're hoping to optimize for me, but I'd like to be sure it's an optimization and not simply "i like this syntax better" (which is fine, but I have to maintain this thing, not you... as nicely as I can possibly make that sentence).

So... what is the optimization we're discussing here?

@nsimeonov

Using the AllKeys property instead of Cast.

I just ran some tests and it really is a bit faster, but both are so close that it's not worth the effort :)

On my machine - 5000 conversions of 200 elements NameValueCollection:

5042.2884 ms for nv.Cast().Select( key => new KeyValuePair( key, nv[ key ] ) ).ToList().ForEach( i => d.Add( i ) );
4993.2856 ms for nv.AllKeys.Select( key => new KeyValuePair( key, nv[ key ] ) ).ToList().ForEach( i => d.Add( i ) );
4844.2771 ms for the following code foreach ( string i in nv ) d.Add( i, nv[ i ] );

Not worth the efforts IMO.

Compliments again for the code and thanks for sharing it.

@robconery
Collaborator

Cheers :). If you feel like forking/pushing, i'd be happy to take the change...

@robconery robconery closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.