140

What convention do you use to comment getters and setters? This is something I've wondered for quite some time, for instance:

/**
 * (1a) what do you put here?
 * @param salary (1b) what do you put here?
 */
public void setSalary(float salary);

/*
 * (2a) what do you put here?
 * @return (2b)
 */
public float getSalary();

I always find I'm pretty much writing the exact same thing for 1a/b and 2a/b, something like 1a) Sets the salary of the employee, 1b) the salary of the employee. It just seems so redundant. Now I could see for something more complex you might write more in the (a) parts, to give context, but for a majority of the getters/setters out there the wording is almost exactly the same.

I'm just curious if, for the simple getters/setters its ok to only fill in either the (a) part OR the (b) part.

What do you think?

ThaDon
  • 7,826
  • 9
  • 52
  • 84
  • 58
    As an aside, please don't use float for anything monetary (such as salary here), ever. See e.g. http://stackoverflow.com/questions/965831/how-to-parse-a-currency-amount-us-or-eu-to-float-value-in-java/965858#965858 – Jonik Jun 22 '09 at 19:45
  • 5
    Use BigDecimal instead. – Josep Apr 04 '13 at 14:03

14 Answers14

195

Absolutely pointless - you're better off without this kind of crap cluttering your code:

/**
 * Sets the foo.
 * 
 * @param foo the foo to set
 */
public void setFoo(float foo);

Very useful, if warranted:

/**
 * Foo is the adjustment factor used in the Bar-calculation. It has a default
 * value depending on the Baz type, but can be adjusted on a per-case base.
 * 
 * @param foo must be greater than 0 and not greater than MAX_FOO.
 */
public void setFoo(float foo);

Especially the explanation of what the property actually means can be crucial in domain models. Whenever I see a bean full of properties with obscure names that only investment bankers, biochemists or quantum physicists understand, and the comments explain that the setGobbledygook() method "sets the gobbledygook.", I want to strangle someone.

Michael Borgwardt
  • 342,105
  • 78
  • 482
  • 720
  • 5
    My sentiments exactly, the worst are the domain specific models where only a domain expert knows what the heck the property means. – ThaDon Jun 23 '09 at 13:20
  • 8
    Even if it is useful what would you do for getFoo(). Will you copy the same comment for the getFoo() as well ? – Vinoth Kumar C M Sep 15 '11 at 14:07
  • 3
    @cmv: Obviously the "param" part would not be copied. I'm undecided whether the value of having the information attached to both accessors directly justifies duplicating the information. Probably yes. Even better would be a way to attach one comment to both; I believe this is available in Project Lombok. – Michael Borgwardt Sep 15 '11 at 14:18
  • @VinothKumar: maybe it would be nicer to simply explain the property in the getter (as in "Foo is the adjustment factor used in the Bar-calculation.") and the effects of changing the value in the setter (or whether it is necessary or not to initialize that value -- in the answer's example it is not necessary to initialize Foo since "it has a default value depending on the Baz type"). – freitass May 04 '13 at 17:05
  • 2
    +1 for "obscure names that only investment bankers, biochemists or quantum physicists understand" – Jackson May 31 '14 at 05:23
  • I absolutely disagree with this answer. And it does not answer the question one bit. The field `foo` has a meaning and it should hold the javadoc comment for this meaning. Getters and setters, well, they get and set the value. One option if to copy/paste the meaning of the field in the getter and setter, but that's duplication. What comment should they hold then ? In my projects I leave the javadoc empty (unless the getter or setter has a specific behavior, such as checking for null), one just has to click on the field to see what we're talking about. Too much duplication. – pieroxy Aug 26 '23 at 12:03
  • @pieroxy: I think you misunderstood the question, and therefore the answer. It's not about whether to put the comment on the field or the accessors, it's about *what* to put in the comment. – Michael Borgwardt Aug 26 '23 at 20:52
99

I usually just fill the param part for setters, and the @return part for getters:

/**
 * 
 * @param salary salary to set (in cents)
 */
public void setSalary(float salary);

/**
 * @return current salary (in cents, may be imaginary for weird employees)
 */
public float getSalary();

That way javadoc checking tools (such as Eclipse's warnings) will come out clean, and there's no duplication.

ThaDon
  • 7,826
  • 9
  • 52
  • 84
sleske
  • 81,358
  • 34
  • 189
  • 227
  • Can you fix the typo? "@return part for setters" – Jonik Jun 22 '09 at 19:41
  • I think I like this answer best as there is no redundancy and things like cobertura won't make it look like the coder is slacking with his/her commenting – ThaDon Jun 22 '09 at 20:04
  • 1
    There's also a typo in salary()'s comment. It's not JavaDoc comment. – Fostah Jun 22 '09 at 20:14
  • 1
    I agree that it is the best approach to commenting accessors. – Fostah Jun 22 '09 at 20:16
  • I think the 's' in 'salary' should be in lowercase here, taking into account how these render in the Javadoc output (they are not at the start of a sentence. This Javadoc glitch is a pet peeve of mine.) – Hakanai Oct 21 '10 at 22:42
  • 36
    Adding noise to your code for the sake of silencing overly pedantic warning from our tools feels wrong to me. If it doesn't add value to a programmer, then the right solution would be to turn down/fix the verbosity of the tools and/or ease up on how much we care about jumping through hoops so that the tools reward us. Analysis tools are supposed to help us and save effort, not create more senseless tasks for us. – Lyle Mar 22 '11 at 21:31
  • An effect of this is that now the alphabetic method summary contains no text at all for these methods. This might be good or bad depending on point of view. – Paŭlo Ebermann Dec 23 '11 at 22:04
  • 2
    @Lyle That may be true, but I feel like there's almost always something useful that a programmer can say which describes a getter/setter better than just the method signature alone. Yes, there are cases where a programmer is lazy and just repeats the method signature in the comment, but I think that generally speaking, it is a helpful behavior to force. – Matt Vukas Sep 22 '14 at 14:12
39

Generally nothing, if I can help it. Getters and setters ought to be self-explanatory.

I know that sounds like a non-answer, but I try to use my time for commenting things that need explanation.

Eric Wendelin
  • 43,147
  • 9
  • 68
  • 92
  • 5
    Another valid answer along these lines might be "designs with getters and setters do not properly understand the notion of encapsulation" :) – Hakanai Oct 21 '10 at 22:44
  • 2
    @Trejkaz: Not true, because accessor methods allow for read-only or write-only properties, and for polymorphism (and so wrapping, proxying, and so on). – Laurent Pireyn Apr 29 '11 at 12:20
  • 2
    They may allow for those things, but often a builder pattern can replace setters (less mutable) or a visitor pattern can replace getters (more flexible.) – Hakanai May 02 '11 at 22:47
  • I certainly like and use the builder pattern, but there's so much support for POJOs (e.g. in Hibernate) that getters and setters still have their very prominent place, for better or for worse. It's the daggiest thing about Java, IMHO, and after writing repetitive JavaDocs for well over a decade, I'm about ready to subscribe to @sleske's advice. – Michael Scheper May 21 '13 at 07:40
35

I'd say only worry about commenting getters and setters if they have some sort of side effect, or require some sort of precondition outside of initialization (i.e.: getting will remove an item from a data structure, or in order to set something you need to have x and y in place first). Otherwise the comments here are pretty redundant.

Edit: In addition, if you do find a lot of side effects are involved in your getter/setter, you might want to change the getter/setter to have a different method name (ie: push and pop for a stack) [Thanks for the comments below]

Carl Manaster
  • 39,912
  • 17
  • 102
  • 155
Gopherkhan
  • 4,317
  • 4
  • 32
  • 54
  • 10
    arguably, you should change the name of getters that have side effects to be more clear, as not all developers will read the comments. – akf Jun 22 '09 at 19:39
  • That's fine - but that requires users of your API to know that, had there been any side effects, they *would have been documented* ! – oxbow_lakes Jun 22 '09 at 19:41
  • akf, I was thinking exactly that after posting :) I guess I'll add it to my response. – Gopherkhan Jun 22 '09 at 19:49
  • 1
    but if you don't document "stupid" getters and setters (it's what I prefer, too!) - how do you get rid of Eclipse warnings on missing javadoc? I don't want to clutter my workspace with warnings like that, but I also don't want that warning to be disabled for all other methods... – Zordid Jun 14 '12 at 13:50
13

Ask yourself what do you want people to see when the comments are viewed as JavaDocs (from a browser). Many people say that documentation is not necessary since it's obvious. This won't hold if the field is private (unless you explicitly turn on JavaDocs for private fields).

In your case:

public void setSalary(float s)
public float getSalary()

It's not clear what salary is expressed in. It is cents, dollars, pounds, RMB?

When documenting setters/getters, I like to separate the what from the encoding. Example:

/**
 * Returns the height.
 * @return height in meters
 */
public double getHeight()

The first line says it returns the height. The return parameter documents that height is in meters.

Steve Kuo
  • 61,876
  • 75
  • 195
  • 257
  • 1
    while I agree with you, I think that one must make sure that the function comments are not making out for a bad chosen, non-explicit function name. – Gad Aug 31 '10 at 11:54
  • I'm a big supporter of JavaDocs, but also a big supporter of self-documenting code. So for the setter at least, I'd do something like `public void setSalary(float aud)` (or more realistically, `public void setSalary(BigDecimal aud)`). Better yet, the property ought to be of type `abstract class CurrencyAmount`, which in turn has the properties `java.util.Currency currency` and `java.math.BigDecimal amount`. Most developers I've worked with are terribly lazy with JavaDoc, but enforcing API like this makes this less of a problem. – Michael Scheper May 21 '13 at 07:51
  • If the unit is a SI unit like meters / seconds, there is less need to document, if it is not an Si unit then it must be documented or better named to include the non standard unit, e.g heightFeet – AlexWien Jun 21 '13 at 18:56
8

Why don't they just include a reference tag to let you comment the field value and the reference from getters and setters.

/**
* The adjustment factor for the bar calculation.
* @HasGetter
* @HasSetter
*/
private String foo;

public String getFoo() {
  return foo;
}

public void setFoo() {
  this foo = foo;
}

So that the documentation applies to the getter and setter as well as the field (if private javadocs is turned on that is).

Andrew Thompson
  • 168,117
  • 40
  • 217
  • 433
Steve
  • 89
  • 1
  • 1
6

This kind of boilerplate can be avoided by using Project Lombok. Just document the field variable, even if it's private, and let Lombok annotations generate properly documented getters and setters.

For me, this benefit alone is worth the costs.

Michael Scheper
  • 6,514
  • 7
  • 63
  • 76
5

I'm really disappointed about the answers basically saying comprehensive documenting is a waste of time. How are clients of your API supposed to know that a method called setX is a standard JavaBean property setter unless you say so clearly in the documentation?

Without documentation, a caller would have no idea whatsoever if the method had any side effects, other than by crossing their fingers about the apparent convention being followed.

I'm sure I'm not the only one here to have had the misfortune to find out the hard way that a method called setX does a whole lot more than just set a property.

oxbow_lakes
  • 133,303
  • 56
  • 317
  • 449
  • 11
    Without documentation, any caller would assume that a method called setX sets X. It follows that if setX actually sets X, without doing anything else important, then you don't need documentation. – mqp Jun 22 '09 at 19:42
  • That's great! Now does this company CrudTech, whose API I am coding against, follow your convention, or does it follow someone else's on this thread? Hmmmm – oxbow_lakes Jun 22 '09 at 19:47
  • 5
    There is no point in writing "sets the price" in the setPrice doc if the method just sets the value for the price property, but if it also e.g. updates the totalPrice property and recalculates the tax, such behaviour should obviously be documented. – João Marcus Jun 22 '09 at 20:32
  • No; but "JavaBean property setter" is perfectly descriptive – oxbow_lakes Jun 22 '09 at 20:43
  • 8
    You seem to be asking for the documentation to state "This does what you expect." Which is a bit like writing "Caution: HOT" on a cup of coffee. In a perfect world, there would be no need to ever say such things. – Kevin Panko Apr 06 '10 at 21:09
  • 1
    Yes - having used APIs where methods called things like `setX` had side-effects other than the expected, I can indeed state with confidence that this is not a perfect world. – oxbow_lakes Apr 06 '10 at 22:33
3

If there isn't special operation in getter/setter I usually write :

With Javadoc (with private option):

/** Set {@see #salary}. @param {@link #salary}. */
public void setSalary(float salary);

and/or

/** 
 * Get {@see #salary}.
 * @return {@link #salary}.
 */
public float salary();

With Doxygen (with private extract option):

/** @param[in] #salary. */
public void setSalary(float salary);

/** @return #salary. */
public float salary();
TermWay
  • 69
  • 1
  • 5
  • 3
    The problem with this approach is that Javadoc doesn't generate the private documentation by default! In that case reference tag `{@see #salary}` is invalid in generated documentation. – Jarek Przygódzki Nov 15 '12 at 08:04
1

Don't put anything if the field name is suficiently descriptive of the contents.

Generally, let the code be self standing, and avoid commenting if at all possible. This may require refactoring.

EDIT: The above refers to getters and setters only. I believe anything non trivial should be properly javadoc'ed.

Thorbjørn Ravn Andersen
  • 73,784
  • 33
  • 194
  • 347
1

Commenting accessors, especially if they don't do any operations anywhere, is unnecessary and a waste of fingertips.

If someone reading your code can't understand that person.getFirstName() returns the first name of a person, you should try everything in your powers to get him fired. If it does some database magic, throws a few dice, calls the Secretary of First Names to get the first name, It's safe to assume it's a non-trivial operation, and document it well.

If, on the other hand, your person.getFirstName() doesn't return a person's first name... well, let's not go there, shall we?

Henrik Paul
  • 66,919
  • 31
  • 85
  • 96
  • 7
    What if getFirstName() returns null? Where would that be documented? – Steve Kuo Jun 22 '09 at 19:44
  • How about security.getFinalMaturity()? Not all property names have an immediately understandable meaning. Would you want to be fired for not knowing what that means? – Michael Borgwardt Jun 22 '09 at 19:44
  • What if the method is implemented by swizzling? How are you supposed to know that unless it's been clearly documented? How are you supposed to know it is a standard setter unless the doc says it is? – oxbow_lakes Jun 22 '09 at 19:45
  • 2
    get/set should in my opinion be reserved for getters and setters. Database lookups should be named like "lookupPerson" or so. – Thorbjørn Ravn Andersen Jun 22 '09 at 21:33
0

it is ok to fill in the (b) part, especially if you put a comment at the field declaration indicating what the field is all about.

akf
  • 38,619
  • 8
  • 86
  • 96
  • No good - people don't read the field comments. Javadoc doesn't even generate the private documentation by default, and IDEs don't show you the field documentation when you use quick help on a method call. – Hakanai Oct 21 '10 at 22:45
  • people don't read the field comments unless they need to. Once there is a need, the more information the better. – akf Oct 22 '10 at 01:06
0

If the javadoc does not add anything, I delete the javadoc and use the auto-generated comments.

Alex B
  • 24,678
  • 14
  • 64
  • 87
0

I always fill in both. The additional time spent typing is negligible, and more information is better than less, in general.

Paul Sonier
  • 38,903
  • 3
  • 77
  • 117