This is an old question, but I'm going to post here anyway. I have some 20+ years of programming and dealing with other people's code.
I think that naming your variable with a short indication as to their scope is really really useful for the next person (or yourself) who will look at your code.
One does not already look at code in an IDE with pretty colors (and I can't remember what the colors means and different IDE show different colors, etc).
True, methods should be short enough so it's not loaded with tons of variables and tons of code but even on short one - when you look at code that is totally unfamiliar, it is sometimes hard to tell whether a variable is a class variable, local variable or method parameter.
To be able to distinguish at a glance makes it very easy to review the code you are unfamiliar with.
Take this example:
public <T> Page<T> moreLikeThis(MoreLikeThisQuery query, Class<T> clazz) {
int startRecord = 0;
ElasticsearchPersistentEntity persistentEntity = getPersistentEntityFor(clazz);
String indexName = isNotBlank(query.getIndexName()) ? query.getIndexName() : persistentEntity.getIndexName();
String type = isNotBlank(query.getType()) ? query.getType() : persistentEntity.getIndexType();
Assert.notNull(indexName, "No 'indexName' defined for MoreLikeThisQuery");
Assert.notNull(type, "No 'type' defined for MoreLikeThisQuery");
Assert.notNull(query.getId(), "No document id defined for MoreLikeThisQuery");
MoreLikeThisRequestBuilder requestBuilder = client.prepareMoreLikeThis(indexName, type, query.getId());
if (query.getPageable() != null) {
startRecord = query.getPageable().getPageNumber() * query.getPageable().getPageSize();
requestBuilder.setSearchSize(query.getPageable().getPageSize());
}
requestBuilder.setSearchFrom(startRecord);
if (isNotEmpty(query.getSearchIndices())) {
requestBuilder.setSearchIndices(toArray(query.getSearchIndices()));
}
if (isNotEmpty(query.getSearchTypes())) {
requestBuilder.setSearchTypes(toArray(query.getSearchTypes()));
}
if (isNotEmpty(query.getFields())) {
requestBuilder.setField(toArray(query.getFields()));
}
if (isNotBlank(query.getRouting())) {
requestBuilder.setRouting(query.getRouting());
}
if (query.getPercentTermsToMatch() != null) {
requestBuilder.setPercentTermsToMatch(query.getPercentTermsToMatch());
}
if (query.getMinTermFreq() != null) {
requestBuilder.setMinTermFreq(query.getMinTermFreq());
}
if (query.getMaxQueryTerms() != null) {
requestBuilder.maxQueryTerms(query.getMaxQueryTerms());
}
if (isNotEmpty(query.getStopWords())) {
requestBuilder.setStopWords(toArray(query.getStopWords()));
}
if (query.getMinDocFreq() != null) {
requestBuilder.setMinDocFreq(query.getMinDocFreq());
}
if (query.getMaxDocFreq() != null) {
requestBuilder.setMaxDocFreq(query.getMaxDocFreq());
}
if (query.getMinWordLen() != null) {
requestBuilder.setMinWordLen(query.getMinWordLen());
}
if (query.getMaxWordLen() != null) {
requestBuilder.setMaxWordLen(query.getMaxWordLen());
}
if (query.getBoostTerms() != null) {
requestBuilder.setBoostTerms(query.getBoostTerms());
}
SearchResponse response = requestBuilder.execute().actionGet();
return resultsMapper.mapResults(response, clazz, query.getPageable());
}
Now, time yourself and look at the code (extracted from ElasticsearchTemplate from the spring-data-elasticsearch project - the code I was reviewing which prompted me to search on Google for what people say about naming conventions).
- What's the scode of
resultsMapper
?
- Is
requestBuilding
a parameter?
- etc...
Here is my simple suggestion on how variables should be named:
- Class static attributes (i.e. constants): ALL_CAPS_WITH_UNDERSCORES (e.g.
HOST_NAME
).
- Class attributes (i.e. class instance variables): camelCase (e.g.
resultsMapper
).
- Method parameters: prefixed with
a
(e.g. aQuery
, aClazz
).
- Local variables: prefixed with
my
(e.g. myIndexName
, myType
).
The code above becomes:
public <T> Page<T> moreLikeThis(MoreLikeThisQuery aQuery, Class<T> aClazz) {
int myStartRecord = 0;
ElasticsearchPersistentEntity myPersistentEntity = getPersistentEntityFor(aClazz);
String myIndexName = isNotBlank(aQuery.getIndexName()) ? aQuery.getIndexName() : myPersistentEntity.getIndexName();
String myType = isNotBlank(aQuery.getType()) ? aQuery.getType() : myPersistentEntity.getIndexType();
Assert.notNull(myIndexName, "No 'indexName' defined for MoreLikeThisQuery");
Assert.notNull(myType, "No 'type' defined for MoreLikeThisQuery");
Assert.notNull(aQuery.getId(), "No document id defined for MoreLikeThisQuery");
MoreLikeThisRequestBuilder myRequestBuilder = client.prepareMoreLikeThis(myIndexName, myType, aQuery.getId());
if (aQuery.getPageable() != null) {
myStartRecord = aQuery.getPageable().getPageNumber() * aQuery.getPageable().getPageSize();
myRequestBuilder.setSearchSize(aQuery.getPageable().getPageSize());
}
myRequestBuilder.setSearchFrom(myStartRecord);
if (isNotEmpty(aQuery.getSearchIndices())) {
myRequestBuilder.setSearchIndices(toArray(aQuery.getSearchIndices()));
}
if (isNotEmpty(aQuery.getSearchTypes())) {
myRequestBuilder.setSearchTypes(toArray(aQuery.getSearchTypes()));
}
if (isNotEmpty(aQuery.getFields())) {
myRequestBuilder.setField(toArray(aQuery.getFields()));
}
if (isNotBlank(aQuery.getRouting())) {
myRequestBuilder.setRouting(aQuery.getRouting());
}
if (aQuery.getPercentTermsToMatch() != null) {
myRequestBuilder.setPercentTermsToMatch(aQuery.getPercentTermsToMatch());
}
if (aQuery.getMinTermFreq() != null) {
myRequestBuilder.setMinTermFreq(aQuery.getMinTermFreq());
}
if (aQuery.getMaxQueryTerms() != null) {
myRequestBuilder.maxQueryTerms(aQuery.getMaxQueryTerms());
}
if (isNotEmpty(aQuery.getStopWords())) {
myRequestBuilder.setStopWords(toArray(aQuery.getStopWords()));
}
if (aQuery.getMinDocFreq() != null) {
myRequestBuilder.setMinDocFreq(aQuery.getMinDocFreq());
}
if (aQuery.getMaxDocFreq() != null) {
myRequestBuilder.setMaxDocFreq(aQuery.getMaxDocFreq());
}
if (aQuery.getMinWordLen() != null) {
myRequestBuilder.setMinWordLen(aQuery.getMinWordLen());
}
if (aQuery.getMaxWordLen() != null) {
myRequestBuilder.setMaxWordLen(aQuery.getMaxWordLen());
}
if (aQuery.getBoostTerms() != null) {
myRequestBuilder.setBoostTerms(aQuery.getBoostTerms());
}
SearchResponse myResponse = myRequestBuilder.execute().actionGet();
return resultsMapper.mapResults(myResponse, aClazz, aQuery.getPageable());
}
Is that perfect? I don't think so. But the above, as far as variables are concerned, is now easier to read. There are other things such as alignment and spacing, which I won't get into in this answer as it is not related to the question, which would make it easier to read as well.
You don't like Camel Case? Fine, use underscores, etc, but prefix your local variables and your parameters to make them different than class instance variables.
You don't like a
and my
- fine, just stay consistent within your project and use something else... but do use something.
Rule #1: consistency within the project.
Rule #2: make it easy to read and don't require the reader to know everything before he can learn.