Enumerate SqlDataReader Columns

Question

Given the following code snip:

using (var reader = cmd.ExecuteReader())
{
    while(reader.Read())
    {
        var count = reader.FieldCount; // the first column must be name all subsequent columns are treated as data
        var data = new List<double>();
        for (var i = 1; i < count; i++)
        {
            data.Add(Convert.ToDouble(reader[i].ToString()));
        }
        barChartSeries.Add(new ColumnBarChart.ColumnChartSeries((string)reader[0],
                                                                data));
        columnChart.xAxis.categories.Add((string)reader[0]);
    }
}

Is there an easy way to eliminate the for loop? Perhaps using linq?

reader[0] will always be a string reader[0+?] will be a double

I want to pull all the doubles into a list if possible.

Why do you want to eliminate the loop here? I would get rid of the double-to-string-to-double conversion (use GetDouble instead), but the rest seems okay to me... — Jon Skeet
– Jon Skeet, Commented Jul 11, 2013 at 3:19
@JonSkeet Good point - I dislike looping is all. This will be pull a HighChart object stored as json, deserializing the object, then querying the database (potentially a different db) based upon the conStr identifier in the object, then parsing the data and then re-serializing to json for returning in a REST service. Speed is somewhat of a concern I suppose. — Wjdavis5
– Wjdavis5, Commented Jul 11, 2013 at 3:23
You could replace the for with Enumerable.Range and do it with Linq, but basically you are just swapping a for with a foreach in that case, your code seems fine to me, I can see no benefit of refactoring out the for loop — sa_ddam213
– sa_ddam213, Commented Jul 11, 2013 at 3:23
@Wjdavis5, Jon is right, LINQ won't give you any benefits. Besides using GetDouble there's only one thing to do: init your list with count - 1 as capacity to reduce reallocations, or even use fixed-size array instead. Also, use GetString for column 0. — Alex Skalozub
– Alex Skalozub, Commented Jul 11, 2013 at 3:28

Jon Skeet · Accepted Answer · 2013-07-11 03:28:38Z

6

Speed is somewhat of a concern I suppose.

Then you're focusing on entirely the wrong problem.

Out of the things you're doing here, looping is the least inefficient part. More worrying is that you're converting from a double to string and then back to double. At least fix your code to:

for (var i = 1; i < count; i++)
{
    data.Add(reader.GetDouble(i));
}

You could create the list with the known size, too:

List<double> data = new List<double>(count - 1);

Even then, I strongly suspect that's going to be irrelevant compared with the serialization and database access.

Whatever you do, something is going to be looping. You could go to great lengths to hide the looping (e.g. by writing an extension method to iterate over all the values in a row) - but really, I don't see that there's anything wrong with it here.

I strongly advise you to avoid micro-optimizing until you've proven there's a problem. I'd be utterly astonished if this bit of code you're worrying about is a bottleneck at all. Generally, you should write the simplest code which achieves what you want it to, decide on your performance criteria and then test against them. Only move away from simple code when you need to.

answered Jul 11, 2013 at 3:28

Jon Skeet

1.5m893 gold badges9.3k silver badges9.3k bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Wjdavis5 Over a year ago

thank you. i understand what you are saying. i appreciate the words of wisdom.

sa_ddam213 · Accepted Answer · 2013-07-11 03:28:10Z

2

The only way I can see to remove the forloop is to use Enumerable.Range.

But anyway you could do something like:

   var data = new List<double>(Enumerable.Range(1, count).Select(i => reader.GetDouble(i)));

But I see no benefit to this approch, and it just makes the code unreadable

answered Jul 11, 2013 at 3:28

sa_ddam213

43.7k7 gold badges105 silver badges112 bronze badges

Comments

user1968030 · Accepted Answer · 2013-07-11 03:22:53Z

0

I dont think you can loop on it because

public class SqlDataReader : DbDataReader, IDataReader, IDisposable, IDataRecord

and this class does not implement IEnumerable

answered Jul 11, 2013 at 3:22

user1968030

2 Comments

TheEvilPenguin Over a year ago

SqlDataReader extends DbDataReader, which implements IEnumerable. It just doesn't implement IEnumerable<T>, but you can use Cast<T> on it

user1968030 Over a year ago

I don't have have any thing when jon skeet answer you question ;)

TheEvilPenguin · Accepted Answer · 2013-07-11 03:33:24Z

0

You could use the LINQ Cast method to get something you can use other LINQ methods on, but I agree with the other posters - there's nothing wrong with a for loop. LINQ methods will just use a less efficient loop in the background.

I believe this should work, but haven't set up a data set to test it.

reader.Cast<Double>().Skip(1).ToList();

answered Jul 11, 2013 at 3:33

TheEvilPenguin

5,7022 gold badges30 silver badges48 bronze badges

Collectives™ on Stack Overflow

Enumerate SqlDataReader Columns

4 Answers 4

1 Comment

Comments

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

1 Comment

Comments

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related