I have some piece of code that I need your opinion of:
It is deliberately "elongated" to show a bit of detail. Here we go then
Thats the method. The aim of which is to authenticate users. What I had in mind is this:
1. The username is unique 2. Each username must be accompanied by a password 3. The passwords may not necessarily have to be unique
So if a user supplies a username and a password, there maybe one and only one match for the username and password supplied. If there is a match, only one result row gets returned. And if one row is returned, we have a "true" boolean value, else we have a "false".
Thus my question is, is this the best way to go about it?
well, if this is like a review, here are my nickels:
1. The validation to check if the username and passwords are not null should be done at the point where the user keys them in, either in the UI page or the class that calls this method. If this validation is done here to double-check, then it is not ideal to throw a NullPointerException. You would rather want to create an Exception class named like 'AuthenticationException' and throw it, so you know how to handle it specifically where it is caught.
2. The username is unique and this constraint should be enforced not at the program level, but at the database level. Make the 'username' field primary key in the 'users' table.
3. It is nice to also report which of the two - username or password is wrong. i.e. first check to see if username is right and then check if passwords match.
4. Force the method to throw an Exception that would and should occur if authentication fails. That way, you can ensure there is no authentication leak by handling it.
5. If possible, try to move the backend calls to a different method or backend class (am assuming this is business layer).
I guess the modified code would look something like this:
[ July 08, 2008: Message edited by: Sravan Kumar ]
Sravan: 3. It is nice to also report which of the two - username or password is wrong. i.e. first check to see if username is right and then check if passwords match.
Generally, as an authentication error people will say "username/password does not match." If you say that username is valid but password does not match then it is like a hint: "dude you have cracked the username, just guess the password and you are through " [ July 08, 2008: Message edited by: Nitesh Kant ]