Don’t worry, the connection will be closed by using
. Good, because if it hadn’t been used, there would have been more serious problems with the code. I don’t know if you have other problems because I haven’t seen everything, I can only talk about this method in isolation.
The way I was making it was wrong because nothing can be executed after one return
unconditional, which is the case of try-catch
. So how can an execution be done after it’s already out of the method? Usually when you want to ensure that the execution is always done before exiting the method you can use a try-finally
, which is exactly what the using
makes for resources that need to be closed.
But something I find strange is the exception being responsible for determining whether the user can log in or not, it seems very wrong, or at least not robust, even more capturing Exception
that in this case it is not only risky, it seems to be quite wrong, to be almost certain that something wrong will happen and cause undesirable faults. I would take this.
And wouldn’t wear Boolean
This is not C# "real", prefer bool
:)
Ideally the OleDbDataReader
should be "protected" by using
also. It suffers from the same connection problem, it is an external resource that needs to be closed.
Besides, I’m worried about the variable cmd
not be declared within this method. There may be a reason for this, but I doubt it is the right one.
I will understand that the exception is being used to detect whether the user is logged in or not. If this is not the case you have no reason to take this exception and return the result.
I’ll put a shape that looks better:
public bool LogarUsuario(string Pnome, string Psenha) {
using (var conexao = new OleDbConnection(Conexao.stringConn)) {
conexao.Open();
var cmd = new OleDbCommand("SELECT * FROM Usuario WHERE Nome = ? AND Senha = ?", conexao);
cmd.Parameters.Add("@Nome", OleDbType.WChar).Value = Pnome;
cmd.Parameters.Add("@Senha", OleDbType.WChar).Value = Psenha;
using (OleDbDataReader reader = cmd.ExecuteReader()) {
reader.Read();
if (reader.HasRows) {
Nome = reader[1];
}
return reader.HasRows;
}
}
}
I put in the Github for future reference.
Where are you giving the error? Try to be as specific as possible. You can improve your question by clicking [Edit].
– Jéf Bueno
The execution will never reach the
conexao.Close();
, because it runs Return anyway before that.– Aesir
@Matthew what is the intention to return true or false? Is to say if the user is logged, or say that the method performed right or not?
– Maniero
@Matthew Did any of the answers solve the problem? Do you think you can accept one of them? See [tour] how to do this. You’d be helping the community by identifying the best solution. You can only accept one of them, but you can vote for anything on the entire site.
– Maniero