Default comments and meta-information in Java classes/interfaces

In some codebases I saw comments that can be described as default comments. These comments you normally will find in every file of the project and, I believe, in most cases they are automatically generated with the help of IDE. Meta-information is a little bit different. It's actually part of the code. I think I'd better show this on example. This is our test subject (taken from real life and simplified):

public class UserServiceImpl implements IUserService {

    ////////////////////////////////////////////////////////////////////////
    // Constants
    ////////////////////////////////////////////////////////////////////////

    /** Logger for t开发者_StackOverflow中文版his class */
    @SuppressWarnings("unused")
    private static final Log LOG = LogFactory.getLog(UserServiceImpl.class);

    ////////////////////////////////////////////////////////////////////////
    // Attributes
    ////////////////////////////////////////////////////////////////////////

    /** User DAO */
    private IUserDao userDao;

    ////////////////////////////////////////////////////////////////////////
    // Constructors
    ////////////////////////////////////////////////////////////////////////

    /**
     * Default constructor
     */
    public UserServiceImpl() {
    }

    public UserServiceImpl(final IUserDao userDao) {
        this.userDao = userDao;
    }

    ////////////////////////////////////////////////////////////////////////
    // Getter and Setter methods
    ////////////////////////////////////////////////////////////////////////

    /**
     * @return value of {@link #userDao} field
     *
     */
    public IUserDao getUserDao() {
        return userDao;
    }

    /**
     * Sets {@link #userDao} field
     *
     * @param userDao User DAO
     */
    public void setUserDao(final IUserDao userDao) {
        this.userDao = userDao;
    }

    ////////////////////////////////////////////////////////////////////////
    // Implemented/Overridden methods
    ////////////////////////////////////////////////////////////////////////

    /**
     *
     * @see IUserService#saveUser(User)
     */
    @Override
    public void saveUser(final User user) {
        fillMissingFields(user);
        userDao.saveUser(user);
    }

    /**
     *
     * @see IUserService#getUserById(Integer)
     */
    @Override
    public List<User> getUserById(final Integer id) {
        return userDao.getUserById(id);
    }

    /**
     *
     * @see IUserService#getUserList(IEnvironmentContext)
     */
    @Override
    public List<User> getUserList(final @SuppressWarnings("unused") IEnvironmentContext context) {
        return userDao.getUserList();
    }

    ////////////////////////////////////////////////////////////////////////
    // Helper methods
    ////////////////////////////////////////////////////////////////////////

    private void fillMissingFields(final User user) {
        user.setLastUpdated(new Date());
    }

    ////////////////////////////////////////////////////////////////////////
    // toString() method
    ////////////////////////////////////////////////////////////////////////

    /**
     *
     * @see Object#toString()
     */
    @Override
    public String toString() {
        return "UserServiceImpl {...}";
    }
}

This class contains a lot of concepts I want to discuss, so I split them in these types:

1) Section default comments - for each section of the class there is one 3-line comment (like constants, constructors, ...). Please note, that I'm not talking about logical sections of class (like // user managenet or // Account Balance calculation).

2) Getter and Setter default comments - comments for set/get methods that just tell that correspondent method sets of returns field value.

3) Meta comments - comments that describe meaning of some java language constructs. Examples from above: @see IUserService#saveUser(User) - tells that method is overridden/implemented and location of parent method, Default constructor - tells that it's default constructor of Java class, Logger for this class.

4) @SuppressWarnings("unused") - in my concrete example it used to say, that LOG is not used in the class (LOG is really never used in the class, but IDE will not show warning) and context argument is not used, but it's OK (assuming that context is some general information and it's generally completely normal if implementations do not use it).

5) I prefix for interfaces - prefix tells, that this is interface.

6) final for method arguments - prevents code in method body to change it's value

I would like to know your opinion about default comments and meta-information in classes. In order to make it more straightforward I propose you to vote for each type with grades from +5 down to -5:

+5 - I think it's must and it's should be done by every Java developer and should even be enforced with tools (like checkstyle)

...

0 - I don't care. If somebody will tell me to do it, I will - these comments will not bring any positive or negative value.

...

-5 - I strongly discourage everybody to make this. Such default comments/meta-information should be deleted from class as soon as you see it.

I also strongly believe, that it's very important always to explain you option and answer question: why do you think so? (I personally try always to follow this rule). So I also encourage you to explain points you have given for particular type.

I will accept answer with most SO up-votes in about one week.

PS.: Some of you may think that answer is self-evident, but believe me, each of us is very different and something that is self-evident for you can be surprise for me and vice versa. So I encourage you to participate this discussion anyway. (I will appreciate this)

---

The purpose of code documentation is to help anyone reading the code understand it. This and nothing more. This is the goal you should set your sights on and make sure you never lose it. The value of documentation can be expressed as (<the time it would take me to read the code without the documentation> - <the time it takes me to read the code with the documentation>) * <my salary>) Subtract from it the time it took you to write the documentation multiplied by your salary and you get the net value. If that number is likely to be negative, don't bother with writing it. These formulas are obviously purely theoretical, but they allow us to construct a few questions.

Did the comments in the sample code helped me understand the code? Not a single bit. The information in them should be blindingly obvious for anyone who has a bit of experience in Java.

Did they hinder me? Yes, inasmuch as it took me three times longer to scroll through it than it would've been without the comments.

If the code was to be changed, how hard would it be to keep the comments consistent? I believe, this is the clincher. Over-elaborate comment conventions are more likely to be broken, and that's the worst that could happen: a comment which is not just superfluous, but misleading. This is like stealing time from the maintenance dev.

All in all, documentation for documentation's sake is wrong. When you weigh up your choices of documentation, you should always have the reader in mind: how is this piece of documentation going to benefit them? If you keep that in mind, you will make the right choices.

---

I think it would not be fear from me to ask this question and did not tell my opinion on this :). Here are my votes.

To make long story short - I'm against default/autogenerated comments. I believe that comments should not describe self-evident things, explain java language constructs/code conventions or contain information, that can be easily retrieved with the help of IDE.

1) -5 (Section default comments)

These comments from one hand trying to describe Java code conventions (or trying to enforce custom class layout of they do not follow conventions) and from another hand are breaking them - for example let's take // Helper methods section: it enforces to place all private methods in this section at the end of the file. This conflicts with Java code conventions (section 3.1.3):

These methods should be grouped by functionality rather than by scope or accessibility. For example, a private class method can be in between two public instance methods. The goal is to make reading and understanding the code easier.

It's very easy to place method in wrong section by mistake - in this case they can become very puzzling.

The worst is when someone decides to have this in ALL classes no matter what. Small 10 lines long classes will become 50+ line classes (see example in question itself)

2) -5 Getter and Setter default comments

I find these even worse than mess. I believe they actually leak information about class internals. The whole point of set/get methods is to hide fact, that they just set and return value of field.

These comments are self-evident and do not add any useful information - only mess in the source code.

We should also take into account how our brain works. With some time it will actually learn to ignore these comments. In this case we can miss something important or useful.

3) -5 Meta comments

Comments like // Logger for this class or // Default constructor are self-evident and doesn't bring any useful value. If somebody really needs such comments to better understand or read code, than I think it's time for him to read some java book or tutorial for beginners.

@see IUserService#saveUser(User) does not make any sense for me (considering all maintenance efforts, that should be made in order to keep these comments current). All major Java IDEs can provide you with this information - they give visible signs that method overrides/implements something you can jump there with shortcut, you normally can also see which method was overridden in tooltip if you want.

Now consider the possibility, that method in parent class (that you have overridden) was removed (but there is still method in parent of your parent class that still has method). In this case you should update these comments in all subclasses in order to keep them in sync. And I can tell you from experience, that at some point most of these comments would be out of sync, and will serve only two purposes: 1 - pollute the code and 2 - puzzle and mislead other developers.

Theoretically you can find ways to keep them in sync (write IDE plugin, create complex code rewriting during build and start it each night, etc...), but I strongly believe that it's just waste of time.

4) -3 @SuppressWarnings("unused")

I think, that @SuppressWarnings("unused") has it's place in some circumstances, but not in the example, shown in the question. LOG annotated because it's never used. But if it's never used, then IMO LOG should be removed from the class. It is private static field, so it can only be used by current class. If class does not use it, that it is mess.

For context it's also pretty useless. context argument is by definition not expected to be used. It represents information that may be somehow useful for subclasses, but nothing more. So I think It's self-evident that context sometimes would not be used, and it's OK. I think IDEA can often recognize such situations and does not shows warnings if you do not use context.

And for me personally it's hard to read signatures of the methods with all these @SuppressWarnings("unused") annotations (especially if there are several of them).

5) -4 I prefix for interfaces

I personally find it ugly and unnatural. If I want interface for user I will rather name it User and implementation will have name UserImpl. I think it can be compared to Uniform access principle. As long as it's called User I don't care whether it's interface of abstract class or concrete class. If author of User interface at some point decides to make it abstract class - I should not make changes to my code, because most probably he will not change name. But it's not the case with IUser.

From the other hand IDE gives us more than enough hints to distinguish between classes and interfaces.

As far as I know this naming comes from Microsoft COM. In C++ they don't have interfaces so they prefix concrete classes with C (like CUser) and interface classes with I (like IUser). In Java we have interfaces and Naming Conventions. And I think we should follow them unless we have really good reason not to.

6) -1 final for method arguments - prevents code in method body to change it's value

Please, don't understand me wrong. I believe, that it's really bad practice to change arguments' values. I try to write my code as much immutable code as I can.

But when I read code, all these finals in method signatures are just mess for me. For me it's self-evident that it should be final. But they make method signature big and hard to read.

I think it should be possible to enforce this rule with tools (like FindBugs or CheckStyle). And I think it's the best way to do it. Often people do not write all these finals themselves, but allow IDE to add them when they save file (for example eclipse can make). But, it you actually changed argument's value (by accident), then IDE just will not insert final for you.