2
\$\begingroup\$

I have this code:

for (int i = 0; i < lst.Count(); i++)
{
    lst[i].ColumnOrder = (short)i;
}

But I was trying to find a way to do it via LINQ or something else with better performance. I'm pretty sure that there are better ways to change the sequence of a column. I use this code in a drag & drop context.

\$\endgroup\$
  • \$\begingroup\$ I know it's kind silly question. But I really got curious about a better way to do it, and couldn't find anything on the web. PS: The title looks a little weird for the context, but I'm not sure on how to put it, if someone have some idea please do so \$\endgroup\$ – Michel Ayres Aug 16 '13 at 17:21
5
\$\begingroup\$

Let's have a look at some common ways to improve collection-related code:

  1. Use LINQ. LINQ is not a good fit here, because you're modifying the collection. LINQ is good for querying, but that's not what you're doing.
  2. Use foreach instead of for. That wouldn't be a good idea here either, because you do need the index.

So, this means your approach is the right one in general. But there are some smaller things I would change about your code:

  1. Use the Count property instead of the Count() extension method.
  2. Change the type of i to short to avoid the cast. (If there was more than short.MaxValue (32767) items in the collection, this would cause a somewhat confusing ArgumentOutOfRangeException instead of InvalidCastException, but I think that's not a big issue.)
  3. Use a better variable name than lst. lst doesn't tell you anything about the contents of the list and shortening variable names like this is a bad idea (especially if you save just one character). The important part is what does the variable contain (columns, or something like that), not that it's List<T>, so that's what should decide how is the variable named.

With that, the code would look something like this:

for (short i = 0; i < columns.Count; i++)
{
    columns[i].ColumnOrder = i;
}

But I was trying to find […] something else with better performance.

You're not going to find that, for loop has pretty much the best possible performance.

\$\endgroup\$
  • 1
    \$\begingroup\$ Good point about the Count() method, I missed that. Using a short for loop variable doesn't avoid all casting, as there will be an implicit cast when it is compared to the int value from the Count property. \$\endgroup\$ – Guffa Aug 16 '13 at 18:28
  • \$\begingroup\$ the lst was just to put it here on the question ^_^ thanks you for the tips and advises. I +1 @Guffa's answer because of the ++ that I forgot too (I can't start i with the value 1 because of the list position (that starts at 0)) \$\endgroup\$ – Michel Ayres Aug 16 '13 at 18:37
  • \$\begingroup\$ @Guffa Yeah, I primarily wanted to avoid explicit casting to have cleaner code. The only potential problem with that implicit cast is slight performance hit, but that's likely negligible. \$\endgroup\$ – svick Aug 16 '13 at 20:13
  • \$\begingroup\$ @svick: Casting between basic value types is not expensive. It's possible that doing arithmetic on short values instead of int vaues has a bigger performance impact, but that is also a very small difference. \$\endgroup\$ – Guffa Aug 16 '13 at 20:37
5
\$\begingroup\$

You can use LINQ to do it, but that will end up as pretty ugly code. LINQ is used to process data and return a result, and that is not what you are doing here, so using LINQ for this would only mean that your code would produce a dummy result and have a side effect doing so.

Well, for completeness, here is some ugly code that misuses LINQ extension methods to loop through the items:

lst.Select((item, i) => item.ColumnOrder = (short)i).Last();

(Note the Last call that is used to pull the result out of the expression, so that it actually loops through the items. Even if you don't use the result, you still have to loop through the entire result to make it complete the loop.)

Your original code is just the simplest way to do it (except for the use of Count() instead of Count as svick mentioned), and is likely to give the best possible performance. There are other alternatives of course that gives almost the same performance. You can use an enumerator to loop through the items, but then you still need a counter for the column order:

int i = 0;
foreach (var item in lst) {
  item.ColumnOrder = (short)(i++);
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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