Why is this bad code?

I was looking at a question on here about confessing your worst code ever written and I am not quite sure, because of my lack of knowledge on why this 开发者_如何学Cis bad code.

public string GetUsername (string userName)
{
    User user = DbLookup.GetUser(userName);
    return user.Username;
}

Is it because, it assumes username will exist and doesn't check for null? Or is there more to it?

https://stackoverflow.com/questions/130965/what-is-the-worst-code-youve-ever-written/191969#191969

---

because it returns the same thing that the user sends as input to the method... Username

---

It doesn't return the same string that was provided. It return the username from the database and the user may or may not exist - thus it could return null. The method name is perhaps incorrect given what it does. Someone in the original post mentioned that it should be CheckIfUsernameExistsAndReturn sorta method name.

---

Besides the given answers, the method is actually named confusingly, now the maintainer has to dig around to figure out what it does.

---

Because he returns the same string he passed into the method.

---

because if user exists, it just returns the same value submitted as method parameter, and if user does not exist, it will throw a null reference exception.

---

As already stated, it is bad code simply due to the fact the method is redundant really. It is returning (assuming User.Username is the same as the parameter) the same value of the parameter. However, as you mentioned another reason it is bad is because it doesn't check that User is null before it attempts to return the Username.

Another potential issue is GetUser may raise an exception that is not being handled in the method (it could indeed be getting handled externally or internally). Just a thought tho...

An improvement would be to return the User object rather than the username itself:

public User GetUser(string username)
{
     try
     {
          return DBLookup.GetUser(username);
     }
     catch (DBLookupException ex)
     {
          // throw exception or handle exception
          return null;
     }
}