Getting user count from database

Question

Is the following code using good practices? All I want to do is get a count of the number of rows which any given user has:

objSQLCommand = New SqlCommand("select count(id) as record_count from table1 where user = @strUser", objSQLConnection)
objSQLCommand.Parameters.Add("@strUser", SqlDbType.VarChar, 3).Value = strUser

objSQLCommand.Connection.Open()
objSQLDataReader = objSQLCommand.ExecuteReader()
objSQLDataReader.Read()

intRecordCount = objSQLDataReader("record_count")

objSQLDataReader.Close()
objSQLCommand.Connection.Close()

score 3 · Answer 1 · answered Jan 14 '11 at 11:04

3

I would suggest:

Converting the command to a stored procedure
Have the SP return the count as its only output
Then use ExecuteScalar (no need for a reader)
Wrap using around the connection and command to dispose properly.

answered Jan 14 '11 at 11:04

amelvin

8,919
4
38
59

I know how to do all that except your last point. How do I dispose properly? – oshirowanen Jan 14 '11 at 11:23
SPs are pre-optimized in the database so should be a little quicker than a command (T&Cs apply, execution plan timings can go up or down, please read the label) and although this example doesn't build up a string to execute it could lead a developer on to that kind of practice (and the obvious sql injection risks). In these circumstances I reckon using Linq or an SP are the safest and quickest options. – amelvin Jan 14 '11 at 12:09
1

@oshirowanen have a quick read of this solution http://stackoverflow.com/questions/60919/is-sqlcommand-dispose-enough - the using command handles disposing of objects (that implement IDisposable) for you. – amelvin Jan 14 '11 at 12:19

score 2 · Accepted Answer · answered Jan 14 '11 at 11:02

2

Since you're returning a single value you could just use ExecuteScalar instead of the reader.

See sample here.

answered Jan 14 '11 at 11:02

Chris W

3,304
3
26
28

score 2 · Answer 3 · answered Jan 14 '11 at 11:15

2

Why not use LINQ to Entities? Your code has a time warp feel to it...

answered Jan 14 '11 at 11:15

awrigley

13,481
10
83
129

+1 @oshirowanen Linq would reduce all your code to something like table1.Where(t => t.user == strUser).Count(); – amelvin Jan 14 '11 at 12:13
@Oshirowanen: LINQ will make your life much easier. – awrigley Jan 14 '11 at 12:58

score 1 · Answer 4 · answered Jan 14 '11 at 11:11

Its not terrible practice, as per what @amelvin and @ChrisW said. And...

you could wrap up your sql query in a Stored Procedure, just two lines are different:

objSQLCommand = New SqlCommand("GetCountByUser", objSQLConnection);
objSQLCommand.CommandType = CommandType.StoredProcedure;

On a design note, you could probably keep the user reference as an ID rather than using a VarChar for each row:

IE

UsersTable
ID       Name               Email
1       Oshirowanen     Oshirowanen@hotmail.com
2        5arx                   5arx@test.com
3        JeffA                jeffattwood@stackexchange.com

RecordTable
ID        UserID    OtherColumns
1234            1    Oshirenin 1
1235            1    Oshirenin 2
1236            2    5arx's column #1
1237            3    Jeff Attwood was here
1238            2    Another one of 5arx's column

So you could call (in stored proc or inline SQL) it with the numeric ID of the user. This approach stops you duplicating the username in every row of the record table.

Getting user count from database

4 Answers4