[Pipeline130-developers] Comments in code

[Chris S. <cs...@uc...>]

I figure, given Mike's earlier message, we should talk a little bit about comments.

Firstly, I think he's totally right that we should avoid block comments inside method bodies unless there's a really good reason for it. I will convert any in-method block comments to regular block comments for the reason Mike stated (to allow block-commenting around the comments for debugging purposes).

More importantly though, I think we need to discuss *what* we comment. For example, in Job.java we have a bunch of methods like this:
    /**
     *
     * @param id ID of the job
     **/
    public void setID(String id)
    {
        this.id = id;
    }

Is this comment necessary? I think absolutely not. It only means readers have to scroll more to find the important stuff. If the name of the method, variable, parameter or return value completely describes what it does (and in most cases it definitely should), then don't bother commenting. 

Here is a method that I wrote last night in WorkflowEngine.java:
    /**
     * Starts all the ready jobs of all workflows. Probably should only be called
     * when resuming from a paused state.
     */
    public void startReadyJobs()
    {
        for (Workflow w : workflows)
        {
            w.startReadyJobs();
        }
    }

In this case, I think it's a good idea to mention that it starts ready jobs of all workflows, so the first sentence is justified. However, I might not have written the comment just to say that. The really important part is that I specified the circumstances this function should be called. The comment serves as a warning to anyone who might feel like calling it later: it might make you think, "Hey, should I really be calling this here?"

Then of course there are the completely obtuse functions. Not all methods can be as simple as startReadyJobs, and not everything can be broken down into tiny, simple parts (but many things can, so please try to do so!). For example, Justin recently committed the method forwardWorkflows in WorkflowEngine.java. It seems to do something kind of complicated... but I have absolutely no idea what that is. "forwardWorkflows" isn't a very meaningful name and I can't figure out what it does just by reading it. This is the case where we definitely do need a "descriptive" comment -- or else a clearer name. Justin, if you could fix that up, it'd be really helpful -- none of us can use your function until we figure out what it does.

I'm not going to touch any comments right now (though I did last night), but I probably will once we have a chance to discuss the issue. What do you guys think?

-Chris