6

Part of code review feedback is suggesting better names for methods and classes. How can I address feedback about class and method names which does not suggest an alternative?

For example, I might create the class

public class FooBar { }

My reviewer would then say, "Try to think of a better name than this." or "This name isn't helpful." without suggesting an alternative name.

I have already thought through the class name and FooBar is really the best I can come up with. How can I actually address the feedback from this review?

gnat
  • 21,442
  • 29
  • 112
  • 288
Freiheit
  • 1,020
  • 8
  • 16
  • 4
    Since you've taken a second chance to come up with another name and couldn't, why not ask for more feedback? Maybe your reviewer just wanted to you put a little more time into it. – JeffO Feb 10 '14 at 20:10
  • you may want to revise the question title to better match your question, such as by adding "without a suggestion". – DougM Feb 10 '14 at 20:26
  • 4
    This sounds like a great opportunity to chat with the reviewer directly and understand what they'd like to see in a name. – Alex Feinman Feb 10 '14 at 20:28
  • piece of cake. In cases like that, I create a ticket in issue tracker _"address review comments"_, add details of what is requested and estimate effort needed to apply my favorite [six techniques](http://programmers.stackexchange.com/a/129968/31260 "'there are six techniques that were proven to work for me: 1) spend a lot of time on inventing names 2) use code reviews 3) don't hesitate to rename 4) spend a lot of time on inventing names 5) use code reviews 6) don't hesitate to rename'"). After that, I let team lead / manager prioritize it and assign as they want – gnat Feb 10 '14 at 20:50
  • 14
    Guys, don't you **talk** to each other at work? – Doc Brown Feb 10 '14 at 21:57
  • 1
    @DocBrown - They just use SE sites to communicate. Some even have their own tags. – JeffO Feb 11 '14 at 11:04
  • 4
    Note that when it is hard to come up with a meaningful name for something, it can be a sign that the thing has an unclear purpose and would benefit from restructuring. – Sebastian Redl Feb 11 '14 at 11:31
  • @DocBrown - In this case I work from home 3 time zones away from the rest of my team at headquarters. We use instant messaging. We use Crucible for code reviews. We get on the phone. Part of this challenge is a lack of face-to-face communication. – Freiheit Feb 11 '14 at 14:26

3 Answers3

12

This is a communication issue between you and the reviewer. A code review is not a one sided "you will do it this way or else" discussion of code. It is a chance for several other developers at your site to see what, if anything, could use improvement. As often as not, during code reviews of other developer's code, I learn something new.

If your reviewer doesn't have a concrete example of how the name of your class/function/method could be named, then they don't belong in the review. Ask the reviewer for an example that would be appropriate.

If you are the reviewer and don't know what the routine is doing, ask the developer what the code does from a high level perspective. Lead/coach them to an answer that is acceptable for you both.

In either case, it should be a discussion point that as a reviewer you lead or as the reviewed you instigate.

Adam Zuckerman
  • 3,715
  • 1
  • 19
  • 27
5

You have two problems. One is that you have difficulty naming things. The other is that you have difficulty communicating about precisely what is wrong with a name. Your colleagues reviewing the code may share this difficulty, or they may be assuming the precision isn't needed because they think you can figure it out on your own. Either way, learning about naming in a more objective way will help you not only with naming, but with the communication aspect as well.

There are books with entire chapters on the topic of naming, and multitudes of websites discussing those chapters. Check out Clean Code, Code Complete, and Refactoring and do google searches like "Clean code naming." Naming is certainly a skill that can be improved using objective measures.

A very high-level summary of Clean Code from the table of contents:

  • Use intention-revealing names. For example, elapsedTimeInDays instead of d.
  • Avoid disinformation. For example, don't use accountList if it's not a List.
  • Make meaningful distinctions. For example, don't use ProductInfo and ProductData to mean different things.
  • Use pronounceable names.
  • Use searchable names. No single-letter names (except as locals in short methods) or numeric literals.
  • Avoid encodings like hungarian, etc.
  • Avoid mental mapping. Don't make the programmer translate to the real meaning in his head.
  • Classes should have noun or noun phrase names.
  • Methods should have verb or verb phrase names.
  • Don't be cute.
  • Pick one word per concept. For example, don't have fetch, retrieve, and get mean the same things in different classes.
  • Don't pun. For example, don't use add to mean different things in different classes.
  • Use terms programmers are familiar with, like pattern names.
  • Use terms that are familiar to people who know the problem domain.
  • Add meaningful context. For example, use addressState instead of just state if you can't easily tell what kind of state it is from the context.
  • Don't add gratuitous context. For example, accountAddress and customerAddress are good names for instances, but not for classes, which should just be named Address.
Karl Bielefeldt
  • 146,727
  • 38
  • 279
  • 479
1

In order to come up with a better name, you have to take the time to understand what the classes and methods do. You'd think during a code review, they would be able to figure it out, but apparently they look at the names and stop right there. They can't tell you what is missing or more accurate.

Seems like your only response is, "This class is Foo because it is a foo which is a _ . What else could I call it?" If they can't give you an answer, you'll just have to leave it.

JeffO
  • 36,816
  • 2
  • 57
  • 124