Any better approaches to doing this in C# 4.0?

Question

foreach (DataRow row in dt.Rows ) 
        avlCols.Add(row.ItemArray[0].ToString().Trim());

What is 'better'?! This looks fine to me. Though I would never use foreach loop's in C# as they are less efficient than regular for-loop's ;) — Vincent Koeman, Jun 09 '11 at 19:49
does `dt.Rows.ForEach(r => avlCols.Add(r.ItemArray[0].ToString().Trim()));` count? — Bala R, Jun 09 '11 at 19:50
@Magnus What don't you think? Just google it: foreach uses more resources/time than for! — Vincent Koeman, Jun 09 '11 at 19:51
@Vincent: readability and maintainability? 1 Dev's time far outweighs the CPU cycles. — p.campbell, Jun 09 '11 at 19:51
@p.cambell A for loop isn't really less readable than a foreach loop imho — Vincent Koeman, Jun 09 '11 at 19:53
Compromise: use `foreach` for readability unless `for` is measurably faster. It rarely matters but when it matters it matters. — Rick Sladkey, Jun 09 '11 at 19:58
@Vincent Like http://stackoverflow.com/questions/1124753/for-vs-foreach-loop-in-c ? I guess you never use linq than either? — Magnus, Jun 09 '11 at 20:01

Justin Niessner · Answer 1 · 2011-06-10T02:34:09.183

4

There's nothing wrong with that code.

If you're looking for a LINQ one-liner to solve the problem, you could try:

// I added ToList since it looks like your original used a list.
// If you only need IEnumerable, then you can leave off that call.
var avlCols = dt.AsEnumerable()
                .Select(r => r.Field<string>(0).Trim()).ToList();

Or (if you simply need to add those items to an existing list):

avlCols.AddRange(dt.AsEnumerable()
                   .Select(r => r.Field<string>(0).Trim()));

For both of those options, you have to make sure you add a reference to the System.Data.DataSetExtensions assembly as well as have access to the System.Linq namespace.

edited Jun 10 '11 at 02:34

answered Jun 09 '11 at 19:49

Justin Niessner

242,243
40
408
536

1

but since OP has `Add()`, the original list could already have some items in it. – Bala R Jun 09 '11 at 19:51
Bala, that's what List.AddRange() is for. – Ian Pugsley Jun 09 '11 at 19:54
@Geek - Make sure you have included the System.Linq namespace in your project. – Justin Niessner Jun 09 '11 at 19:54
@Geek is right, .Rows doesn't implement IEnumerable (see http://stackoverflow.com/questions/10855/linq-query-on-a-datatable). You'll have to use `dt.AsEnumerable()`. – Ian Pugsley Jun 09 '11 at 20:02
If you're using System.Data.DataSetExtensions.dll to get the Table.AsEnumerable() extension method, I would also use the DataRow.Field() method: `r.Field(0).Trim()` – Joel Mueller Jun 09 '11 at 20:34
@Joel - Good call. I've actually never used DataSetExtensions before...learn something new every day. – Justin Niessner Jun 10 '11 at 02:34

Jordão · Accepted Answer · 2011-06-09T20:16:01.700

1

A couple of suggestions:

use good variable names
use the column name instead of the index
use LINQ

An example:

var names = from personRow in personTable.AsEnumerable()
            select personRow["name"].ToString().Trim();

edited Jun 09 '11 at 20:16

answered Jun 09 '11 at 20:03

Jordão

55,340
13
112
144

If you happen to know the index (perhaps you only have one column) it's faster to use the index instead of forcing the DataRow to look up the index from the name once per row. – Joel Mueller Jun 09 '11 at 20:34
Yes it is faster. But until it's a proven _bottleneck_, I'd favor readability and maintainability. And I don't think it would be a bottleneck in a system that (probably) also employs a database. – Jordão Jun 10 '11 at 00:27

score 0 · Answer 3 · answered Jun 09 '11 at 19:51

0

You may be able to get away with:

var avlCols = from row in dt.Rows select row.ItemArray[0].ToString().Trim();

answered Jun 09 '11 at 19:51

Gabe

84,912
12
139
238

Any better approaches to doing this in C# 4.0?

3 Answers3