Using this Keyword to Call Private Methods
code reviews, software development
After a recent code review I noticed a lot of code similar to the following:
this.createAUser();
or
if (this.isValidProfile()) {
Our coding standards allow for using this with fields to make the distinction clear:
private int count;
pubic void setCount(int count) {
this.count = count
}
We adopted The Elements of Java Style for our coding style guide about 2.5 years ago, but this is a case I’d never spent much time thinking about. I find it a lot clearer to write something like:
createAUser()
or
if (isValidProfile()) {
I always assumed this was a well understood convention, but after spending some Googling around I found nothing specifically on idioms with using this with methods.
I think the best evidence in favor of not using the this keyword is with Martin Fowler’s Extract Method pattern. You often create small methods by doing a simple extract of a code fragment on a method that’s getting a little long. And Martin’s example suggests calling the extracted methods with:
printBanner(); printDetails(getOutstanding())
not:
this.printBanner(); this.printDetails(this.getOutstanding())
For me it all comes down to better readability, but we’ll probably have a short meeting with some of the senior developers to toss it around and come to a final decision. If we hadn’t been doing code reviews on a regular basis we might have never addressed this issue.
Ed Gibbs @ November 1, 2006
In a language that supports functions, like C or JavaScript, printBanner() and this.printBanner() are not equivalent!
If there’s a local variable printBanner, the short form will call a function referenced by that variable.
So if you’re coming from C, or moving to another language, adopting the this.foo() convention can be a good thing.
java’s verbose enough without adopting coding standards that make it worse.
Most of our developers are coming out of a Coldfusion or mainframe background, so it’s not like they’re carrying in a convention from somewhere else.
And after spending a lot of time with Ruby this year, Java’s verbosity can be pretty annoying.
The “readability” argument for using this.methodName() seems odd. asaaf’s point is interesting, but Java doesn’t have functions (yet). What else could it be?
I could see an argument for always qualifying your static calls with the classname instead of this.xxx, but again, my IDE shows them differently anyway.
“What else could it be?”
My guess would be simplicity. If you write member variable access and member method access the same way, the code looks more familiar.
I know when I’m a language newbie, I make up conventions like that. I usually lose them after a while.
After talking to the lead developer about it some more, he just invented the rule on his own following the idea of clarifying a field reference with this.
I was curious what the real reason was. Thanks for solving the mystery
With all due respect, this kind of “dilemma” is a waste of energy (and time). Maybe this is why so many people don’t practice code reviews (or at least don’t like it).
Surely you can find more meaningful and important issues to discuss in a code review. Using one convention or another is arbitrary in most cases (and can be verified automatically, if it is important to you).
Use code reviews to discuss material issues which can help the code and the design become better, and the developer to grow as a professional.
See: http://blog.qualityaspect.com/2006/06/27/elements-of-simplicity-working-around-workarounds/
Lidor
Oops… wrong link in the comment above.
Sorry.
The right one is: http://blog.qualityaspect.com/2006/04/05/are-your-code-reviews-effective/
Lidor
There was more of a mentoring role to this question. Many of our developers are about 2-3 years into OO and Java and so they often aren’t sure about small things like using a this keyword. They ask for and want feedback on whether they’re doing things the ‘right’ way.
Of course there isn’t any right way and it comes down to things like common practice, but in this case making a small change leads to more readable code.
The final decision was to not use this keyword to call private methods going forward, but not to bother and go back and do all the busy work to take them out unless time presented itself.
I think a lot of this usage has to do with modern IDEs rather than a conscious stylistic choice; you type “this.” to pop up a list of available methods, then select the one you want. I tend to make the effort to go back and delete the “this.” afterwards, but many can’t be bothered …
Good point, I hadn’t thought about the IDE angle.