Today we want to look at an example I ran into where taking the time to think of good names for everything in your code can save the day.
First I'd like to explain my long absence. The last months of 2009 where obviously very busy for us, but besides that I've been moving to another house (sort of) and I've had a week off to finally have a bit of a rest. But on to more important things.
The Story
For our new application, I was working on some authorization logic in the service layer. We have a class that represents a user. I was implementing a method called IsUserAuthorized. When writing the virtual version of this in the base class, so all other business objects would be able to implement this behavior, I was thinking "I need to have the Guid identifying the user that should be authorized" and I named that parameter userId. Makes sense, right?
The Problem
As I was implementing this method in the User class, I simply used IntelliSense to generate the method stub for me and I started writing code. This is part of what I came up with at first:
var authorisations = from item in context.UserGroupAuthorisation
where context.UserGroupAuthorisation.Any(
i => i.Group.GroupId == item.Group.GroupId && i.Users.UserId == userId)
&& item.Users.UserId == this.UserId
select item;
It might seem a bit complex, but let me explain. What I'm doing here is, because a user is a member of a group, that user can also see other users in that group. So to determine if a user can see the user at hand, I need to know what groups the user is a member of and see if any of these groups overlap with the groups of the user at hand. Basically, if the above query returns anything, the user is allowed to request this user.
The Solution
Now, from both the code and the explanation, you can see that it gets confusing, because we have two users in this story, who fill in different roles in this story. I was reading back my code to determine if I had done this the right way and I couldn't really grasp it. A simple tweak made a world of difference. Look at the revised code:
var authorisations = from item in context.UserGroupAuthorisation
where context.UserGroupAuthorisation.Any(
i => i.Group.GroupId == item.Group.GroupId && i.Users.UserId == requestingUserId)
&& item.Users.UserId == this.UserId
select item;
As you can see, I renamed the parameter userId to requestingUserId. This has a more than expected impact on the readability of this code. Now you can see that our Lambda expression gets all the group authorizations for the requesting user and links them to the group authorizations in the query. Then all that is needed is to take only the authorizations for the user at hand, explicitly called by this.UserId so it's completely different from the parameter requestingUserId.
The Conclusion
As you can see from the above example, having decent names, even for something as small as a method parameter, can make all the difference when it comes to understanding code. May we all come up with better names!
