0

I get an error not all code paths return a value?

 public string Authentication(string studentID, string password) // this line? { var result = students.FirstOrDefault(n => n.StudentID == studentID); //find the StudentID that matches the string studentID if (result != null) //if result matches then do this { //---------------------------------------------------------------------------- byte[] passwordHash = Hash(password, result.Salt); string HashedPassword = Convert.ToBase64String(passwordHash); //---------------------------------------------------------------------------- // take the specific students salt and generate hash/salt for string password (same way student.Passowrd was created) if (HashedPassword == result.Password) //check if the HashedPassword (string password) matches the stored student.Password { return result.StudentID; // if it does return the Students ID } } else //else return a message saying login failed { return "Login Failed"; } }
Improve this question
3
  • 4
    Your design is pretty ugly since you're mixing the magical string Login Failed with student ids. You could use null as return value for failure, or change the return type to something more complex, perhaps some kind of discriminating union. Commented Apr 24, 2012 at 9:42
  • 2
    You already got the answer to your problems. However, I'd recommend you to refactor this bit a bit. Instead of returning Login Failed as a string, you should rather return null, or a complex type that says whether logging in was successful or not, along with the student's ID (or null). Commented Apr 24, 2012 at 9:43
  • possible duplicate of Why am I getting this error: not all code paths return a value? Commented May 1, 2012 at 18:36

5 Answers 5

Reset to default
6

if the result is not null but the result.Password != HashedPassword you're not returning anything.

You should change to something like:

... if (HashedPassword == result.Password) { return result.StudentID; // if it does return the Students ID } return "Invalid Password"; ...
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

4

The problem is that your first if statement doesn't ensure the returning of a value, due to the nested if statement. Imagine you have result set to a value (not null) and your hashed password and supplied password do not match, if you follow that logic through you will fail to hit a return statement.

You should either add an else clause to your nested if statement like so:

if (HashedPassword == result.Password) //check if the HashedPassword (string password) matches the stored student.Password { return result.StudentID; // if it does return the Students ID } else { return "Login Failed"; }

or more desirably, remove the else statement you already have so the function ends with returning the login failed:

if (result != null) { //.... } return "Login Failed";

...with this second approach you do no need to worry about using the else because if all your other conditions are satisfied, the nested return statement will end the function anyway. Try to think of this final return as the default action if any of the authentication steps fail

Another note to make on your code is that it is not ideal practise to be returning a mix of data in such a way. i.e. the result could be a student ID or it could be an error message. Consider creating a dedicated result class with multiple properties that the calling code can check to see the status of the logic validation. A class something like the following would be a good start:

public class LoginResult { //determines if the login was successful public bool Success {get;set;} //the ID of the student, perhaps an int datatype would be better? public string StudentID {get;set;} //the error message (provided the login failed) public string ErrorMessage {get;set;} }

(saying all that though, your calling code already appears to be aware of the studentID anyway)

Improve this answer

Comments

1

remove the else. Just do 

if(result != null) { ... } return "Login Failed";
Improve this answer

Comments

1

you should also return something in case of:

if (HashedPassword != result.Password)

put an else in the inner if

Improve this answer

Comments

-2

i have made some changes in your code. try it.

public string Authentication(string studentID, string password) { var result = students.FirstOrDefault(n => n.StudentID == studentID); var yourVar; if (result != null) { byte[] passwordHash = Hash(password, result.Salt); string HashedPassword = Convert.ToBase64String(passwordHash); if (HashedPassword == result.Password) { //return result.StudentID; yourVar = result.StudenID; // if it does return the Students ID } } else //else return a message saying login failed { yourVar = "Login Failed"; } return yourVar; }
Improve this answer

1 Comment

That's not going to work: 1) the compiler can't infer the type for var yourVar so that won't compile. 2) you're still not initialising yourVar in the case that currently doesn't have a return path, the user-exists-but-wrong-password case

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.