Monday, December 14, 2009

Bad names

I've recently done code review for someone and stumbled on a GeneralUtils.cs file I didn't see before.
"OMG!!!" I said "it is general for sure, and provides some utils right? what kind of utils?"

Him: Grinning as if his hand was caught in the cookie jar. "A bunch of things related to X,Y and Z objects"
Me: "Well, X Y and Z handles 3 different things right?"
Him: "Yes but I didn't think they deserve an independent class"

The name is bad for sure and I got no objection on that. the thing is, it hides a much bigger problem. The object is doing too many things instead of focusing on just one. This means that the Single responsibility principle was violated.
I usually have a small question that clears the issue in a second. ask yourself what does the class/method is doing. if the answer is "It does x AND y" then a re-factor session should visit your code.

Him: "I see your point. I'll split them to 3. I thought of calling the first one XHelper"
Me: "Sounds good. this means that XHelper will help the X class right?"
Him: "No, I don't have an X class"
Me: "Let's suppose you have an X class, X will call XHelper right?"
Him: "Yes. and?"
Me: "So, if the helper is doing all the work it would make more sense to move it into X. if it doesn't, please give a more descriptive name"
Him: "But I don't have an X class!!!"
Me: "Even better! who are you helping?"

No comments:

Post a Comment