I got a problem with an error that say that my datareader is already open.
My code looks like this
public static Users GetByID(int ID, SqlConnection connection)
{
SqlCommand command = new SqlCommand("Select Name, Email, LastLogin, FK_Role_ID from Users where ID=@id");
command.Connection = connection;
command.Parameters.Add(new SqlParameter("id", ID));
SqlDataReader reader = command.ExecuteReader();
if (reader.Read())
{
Users user = new Users();
user.ID = ID;
user.Name = reader.GetString(0);
user.Email = reader.GetString(1);
user.LastLogin = reader.GetString(2);
user.role = Role.GetRoleByID(reader.GetInt32(3), connection);
reader.Close();
return user;
}
else
{
reader.Close();
return null;
}
}
The error occours in the Role.GetRoleByID saying that the datareader command is alreader open. Which is true, but how do I call Role.GetRoleByID with the information from my reader.
I code in c# and ASP.NET
It looks like your Role.GetRoleByID
will try to reuse the connection.
Options:
SqlDataReader
within GetByID
, close that reader, and then call Role.GetRoleByID
(so you've only got one active reader at a time)I'd go with the first option if I were you - or possibly the last. I'd also use a using
statement to close the reader automatically:
private const string GetUserByIdSql =
"Select Name, Email, LastLogin, FK_Role_ID from Users where ID=@id";
public static Users GetByID(int ID, SqlConnection connection)
{
var sql = ;
Users user;
int roleId;
using (var command = new SqlCommand(GetUserByIdSql, connection))
{
command.Parameters.Add(new SqlParameter("id", ID));
using (var reader = command.ExecuteReader())
{
if (!reader.Read())
{
return null;
}
user = new Users
{
Name = reader.GetString(0),
Email = reader.GetString(1),
LastLogin = reader.GetString(2),
};
// Remember this so we can call GetRoleByID after closing the reader
roleID = reader.GetInt32(3);
}
}
user.Role = Role.GetRoleByID(roleID, connection);
return user;
}
As a fourth option - why not just perform the join required by GetRoleByID
in your existing query? That would mean you'd only need one trip to the database.