Reputation: 46651
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 is 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
Upvotes: 4
Views: 634
Reputation: 82136
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;
}
}
Upvotes: 1
Reputation: 117350
Besides the given answers, the method is actually named confusingly, now the maintainer has to dig around to figure out what it does.
Upvotes: 3
Reputation: 13220
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.
Upvotes: 5
Reputation: 146603
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.
Upvotes: 1
Reputation: 6802
because it returns the same thing that the user sends as input to the method... Username
Upvotes: 17