Mike Clark, author of Pragmatic Project Automation, has written an article called "Write Sweet-Smelling Comments," discussing striking a balance between too many comments and too few, along with examples of what he considers horrible technique.

Comments themselves are a sweet smell when they tell you why something was done. When you come across a comment while you're reading code, it should stop you dead in your tracks. The programmer before you felt so compelled to tell you something important—something that couldn't be documented anywhere else—that she left you a comment. Ignore it at your own peril.

What do you think? How much do you comment, when, and why? What's your opinion of the commenting style of others?

Less comments but more content.

Along with methods, if a field has to be commented, I've found it is usually because it is misnamed.

<code>
// Counter that keeps track of login attempts
private int counter;
</code>

Would be better expressed as:
<code>
private int loginAttemptCounter;
</code>

It is funny how the simple DRY concept can keep you on the high road of development.

I also didn't see Mike touch on spell-checking and grammar. While a comment shouldn't be an English thesis paper it should be professionally written. If English is not your strong point have a buddy on the team review your code for errors. Even the best authors lean heavily on editors.

I agree 100% with the article. Wheneveer you place comments on a getter or a setter method, baby Jesus cries.

I agree 100% with the article. Wheneveer you place comments on a getter or a setter method, baby Jesus cries.

Do you comment for yourself, for the next coder, or for the user of your Javadoc?

I find that if you are thinking about the user of the Javadoc, then every field and method, every parameter and return value, every getter and setter will be commented.

In general, writing software uses a lot of time, something like:

20% - software architecture
25% - implementation design
35% - documentation
10% - writing the code
10% - ongoing maintenance

The problem is that most people don't like to spend time on the expensive parts, so it ends up being:

2% - software architecture
5% - implementation design
1% - documentation
60% - writing the code
9999% - ongoing maintenance

Unfortunately it doesn't add up to the same 100%. Once the company realizes that they've spent 10x-50x too much on the project and it's still not working, they start writing the replacement system, but the problem is that they start down the same road:

2% - software architecture
5% - implementation design
1% - documentation
..

You can see the disaster coming, like a train wreck in slow motion.

Peace,

Cameron Purdy
Tangosol, Inc.
Coherence: Cluster your POJOs!

I always write my comments based on the idea that most of the time the person looking at my code will not be me. I worked at a place that would not allow ANY comments in the code. My boss felt the comments could be wrong and mislead the developer, the code should be self documenting. This was the place that also had a method called 'editImage' that would not edit the image but save it.

Once the company realizes that they've spent 10x-50x too much on the project and it's still not working, they start writing the replacement system, but the problem is that they start down the same road

That is, unfortunately, so ever true.

So the question is, is anyone going to use your Javadoc?

I struggle with this one all the time.

I think if you're writing library code, there's little question -- all public members, and probably everything else too, should be commented. The Javadoc is part of your deliverable.

But I work mostly on corporate projects, and every time I write (well my IDE generates) getter/setters, I wonder what the value of commenting them is. It seems that only occasionally is the comment non-trivial and not redundant. I don't find Javadoc output gets used much for internal project code ... people are just going to read the code.

Regards

John Hurst

I agree 100% with the article. Wheneveer you place comments on a getter or a setter method, baby Jesus cries.

:) Nothing makes me cry like seeing "gets the Foo" on a getFoo() accessor.

However, I <em>do</em> document getters/setters by taking the outlook that I am documenting the (induced) bean property like any other piece of data: "The 'foo' property specifies the blah blah blah that this bean has when yada yada yada. If it's null, that means mumble mumble. If it has the special value GNXT, that means such-and-such. Setting this property has the following side effects: ...". And then the getFoo() javadoc says "See {@link #setFoo(Foo)}."

John.

I agree 100% with the article. Wheneveer you place comments on a getter or a setter method, baby Jesus cries.

If a getter or setter doesn't need a comment, it certainly doesn't need to be a method. The entire purpose of such a construct is to say "this property must have something extraordinary done to it before it is stored/retrieved"

If a getter or setter doesn't need a comment, it certainly doesn't need to be a method. The entire purpose of such a construct is to say "this property must have something extraordinary done to it before it is stored/retrieved"

I always put setters and getters around publicly-accessible properties, not for encapsulation (because setters and getters do not really provide that much encapsulation), but for protection against future API changes. If a direct field access in Java the language could turn into a getter / setter call (again, I'm finding myself liking something in C#! That Anders guy! He's smart!), then I'd be all for public fields.

I really only use non-private fields for fields that should be accessed by subclasses, and obviously in situations where subclasses know *lots* about the internals about the superclass. I'd like it if Java had a modifier for allowing subclasses only to access something (and not things that happen to be in the same package).


-Patrick

--
Patrick Linskey
http://solarmetric.com

Hi,

As well as future API protection, some other reasons to use getters/setters for a Java property:

- Allows for a readonly property by providing only a getter.
- Allows the property to be used by many JavaBeans-based APIs, e.g. setter injection.
- Allows the property to satisfy an interface with getFoo() among its methods.
- Better supports the property for interception, e.g. by AOP.

Regards

John Hurst

Hi,As well as future API protection, some other reasons to use getters/setters for a Java property:- Allows for a readonly property by providing only a getter.- Allows the property to be used by many JavaBeans-based APIs, e.g. setter injection.- Allows the property to satisfy an interface with getFoo() among its methods.- Better supports the property for interception, e.g. by AOP.RegardsJohn Hurst

I would add this:

- Allows the property to be changed to a derived property i.e. a property that is not saved to a field but instead computed from other properties.

If a direct field access in Java the language could turn into a getter / setter call (again, I'm finding myself liking something in C#! That Anders guy! He's smart!), then I'd be all for public fields.

Yeah, if Microsoft can invent preemptive multitasking and the graphical UI, they can surely invent accessor methods that look just like public data member references.

Unless Bertrand Meyers does it earlier in Eiffel.

I hope you'll pardon my sarcasm, but this is the sort of thing that just drives me nuts: folks whose first exposure to computers was MS-based, so now they think MS invented everything and is therefore "smart", somehow.

>:(

John.

If a direct field access in Java the language could turn into a getter / setter call (again, I'm finding myself liking something in C#! That Anders guy! He's smart!), then I'd be all for public fields.

Yeah, if Microsoft can invent preemptive multitasking and the graphical UI, they can surely invent accessor methods that look just like public data member references.

If you look carefully at my post, you'll probably find that I didn't say that Microsoft "invented" property-accessor semantics. In fact, I didn't even mention Microsoft at all! And I certainly didn't bring up multitasking or UIs or anything.

I hope you'll pardon my sarcasm, but this is the sort of thing that just drives me nuts: folks whose first exposure to computers was MS-based, so now they think MS invented everything and is therefore "smart", somehow.>:(John.

You might not want to make assumptions about my computing background based solely on the fact that I said something good about C#. For what it's worth, the first computer that I wrote programs for was a TI, and I did Apple, VMS, IRIX, Solaris coding before I ever did any development for a Windows machine. I'm writing this post on an Apple PowerBook G4. My first exposure to computers was certainly not MS-based; in fact, I'm pretty inexperienced when it comes to programming for the MS languages.

I do, however, think that Anders Hejlsberg is a very smart guy. And I think that he, and Microsoft by extension, did some pretty cool stuff with C#.

-Patrick

--
Patrick Linskey
http://solarmetric.com

I did a quick read and I agree. I like the "police tape" description. Most of my comments are police tape.

I always thought arguing about how to write comments was kind of silly... your team should probably be peer-reviewing each other's code, or re-reading your own periodically, and if you have trouble figuring something out, comment it *then*, saying what you just had to figure out or explain to the other person. Most comments written to satisfy some set of rules, instead of actual people, seem worthless to me. What you think is tortured genius when you're writing code, and what you're going to have trouble understanding when you forgot that mindset later, often have no relationship to each other, in my experience.

From the article:

public void authenticateUser() {
    findAuthenticationMethod();
    if (isNetworkConnection()) authenticateHost();
    String password = getPasswordFromUser();
    if (invalidPassword(password)) soundAlarm();
}

I actually find that breaking things up in this manner doesn't necessarily lead to a better code smell, compared to something like:

public void authenticateUser() {
    // find authentication method
    {
        ...
    }

    // authenticate host if necessary
    {
        ...
    }

    // get password from the user
    {
        ...
    }

    // sound alarms as needed
    {
        ...
    }
}

The problem is that the first time you write some code to do something, you are probably not going to delineate the boundaries in the right way for extensibility, reuse etc. So, pretending that things are well-architected for reuse the first time through is often just that -- a pretense. Well-designed method names (i.e., turning the scoping blocks and comments in my example into method names in Mike's) only serve to codify the comment in the method name. It might read like it's better architecture, but it probably won't hold up to the test of time any better.

For example, odds are that the bit where Mike does "if (isNetworkConnection()) authenticateHost();" will eventually evolve into "authenticateTransport()", or maybe something like:

AuthenticationMechanism authMechanism = findAuthenticationMechanism();
authMechanism.authenticateTransport();

Typically, we don't know exactly which bits of code will need to be refactored in which ways. (If we did, we wouldn't really be *refactoring*; we'd just be renaming things here and there.)

Now, it would be absolutely bad to just copy-and-paste that block, or part of that block, somewhere else. But IMO, separating things out into appropriate blocks of reuse is a refactoring that is often best left for when that functionality needs to be reused.

Additionally, prematurely moving the actual implementation logic out of the picture often just makes an algorithm harder for a maintainer to grok. This is not to say that everything should be unrolled all the time, but putting something in a separate method just for the sake of getting it out of view has the side effect of adding a level of obfuscation. This can go horribly wrong if someone adds some bit of behavior to a method without changing the name of the method -- all of a sudden, a cursory read of the high-level does not provide a full picture of what's going on. This is one of the reasons I like using scoping blocks in code. In addition to enforcing isolation between logically separate bits of a method, blocks can be easily folded and unfolded by IDEs, so it is trivial to collapse a blocked-out section of code to get a high-level picture of what's going on.

This is all a balancing act, of course -- methods that are "too long" are certainly suspect. But methods that just call a handful of other methods can be similarly difficult to maintain.

More often than not, I find that the eventual finished code *does* end up with cohesive functionality broken up into separate methods, classes etc., but typically not in ways that I necessarily expected when I first started writing the code.

-Patrick

--
Patrick Linskey
http://solarmetric.com

This can go horribly wrong if someone adds some bit of behavior to a method without changing the name of the method -- all of a sudden, a cursory read of the high-level does not provide a full picture of what's going on.

Well, yes but things can go horribly wrong if someone adds some bit of behavior to a block without changing the comment above the block. Especially if the developer is using the IDE to fold the blocks.

I agree, it's a balance. But another reason for using small methods is that you can see the overall algorithm of a method at a glance, and can more easily see the interaction and dependencies between any parameters. With lots of blocks in one method, you can declare a bunch of variables at the top of the method and have all sort of subtle and hard to spot side effects as the blocks set those variables. With small methods, you're force to explicitly pass in and return stuff, so things are more explicit.

Patrick, you make some good points, but there's a hidden assumption here that I don't buy:

But IMO, separating things out into appropriate blocks of reuse is a refactoring that is often best left for when that functionality needs to be reused.

The assumption is that doing it now makes it harder to do adjust it later, when it's being reused. In my experience, the opposite is true. Even if you don't subdivide the method optimally now, having it broken up that way makes it easier to adjust things later on when the mistakes become clear.

It is a balance, of course, and one problem with tiny, article-sized examples like the one Mike used is that they sometimes fall on the "not worth the trouble" side of the line. But a method in that style (blocks of code, each preceded by an explanatory comment) doesn't have to be much longer or more complicated for me to start extracting methods.

---Glenn

The assumption is that doing it now makes it harder to do adjust it later, when it's being reused. In my experience, the opposite is true. Even if you don't subdivide the method optimally now, having it broken up that way makes it easier to adjust things later on when the mistakes become clear.

I definitely feel a bit of inertia once a bit of code is broken out into a method, especially if I wasn't the one to do the breaking-out. Clearly, it's easy enough to search around within the project to look for other uses. But of course, this all breaks down the minute that the class becomes public and the code gets shipped -- all of a sudden, there's an external-maintenance nightmare waiting to happen (assuming the factored-out methods are not protected or public).

I guess the thing I find the most important about divvying up a block of code into various responsibilities and roles is the contracts between those subdivisions. Just separating out code for the sake of it does not necessarily lead to appropriate subdivisions.

But a method in that style (blocks of code, each preceded by an explanatory comment) doesn't have to be much longer or more complicated for me to start extracting methods.

Certainly. I usually end up breaking up long methods when I can't see them all in my 50-line-by-80-column buffer at the latest. (But sometimes not -- switch statements in parsers are a great example of somewhere that methods end up needing to be long by definition.)

-Patrick

--
Patrick Linskey
http://solarmetric.com

Patrick wrote:

I definitely feel a bit of inertia once a bit of code is broken out into a method, especially if I wasn't the one to do the breaking-out. Clearly, it's easy enough to search around within the project to look for other uses. But of course, this all breaks down the minute that the class becomes public and the code gets shipped -- all of a sudden, there's an external-maintenance nightmare waiting to happen (assuming the factored-out methods are not protected or public).

I practically heard the CLANG! in my head when I read this and realized where we really disagree. We're talking past each other because we spend our time writing different kinds of software. Your product, Patrick, is largely a third-party reusable code library, and such external-maintenance nightmares are part of what you worry about every day.

I typically write internal applications (and libraries for reuse within the same team or small cluster of teams) and such issues aren't a big worry. Oh, they're a concern, of course, but their importance is greatly diminished relative to comprehensibility and flexibility of the codebase.

I see a similar disconnect when I read Josh Bloch's Effective Java. Josh has spent his entire career writing third-party libraries, and that focus colors his thinking. Most of his recommendations are universally applicable, but there are a few that I think make precisely the wrong tradeoff if you're doing corporate IT projects or standalone products.

This isn't to discount your opinion, Patrick. You're right to have different priorities. (Of course, those factored-out methods should be private until that later point comes when they become reused.)

---Glenn

You're absolutely right -- I end up spending lots more time thinking about things like class and interface names than would casually seem appropriate.

Given Java's rather poor packaging / assembly concepts, issues about visibility of methods etc. seem to often end up becoming something of a hassle. For example, you might want a method to be protected such that it can be accessed by a subclass, but you might want to only allow access to that method for things within a given product codebase. Java needs another level of metadata -- something akin to .NET's published interface definition capabilities. I think that's one of the biggest things that .NET got right that we should be stealing back.

-Patrick

--
Patrick Linskey
http://solarmetric.com

I definitely feel a bit of inertia once a bit of code is broken out into a method, especially if I wasn't the one to do the breaking-out.

Well, for me at least, the inertia works the other way. With a class that has lots of small methods I'm more likely to change or refactor one of them, as I can easily see and reason about the parameters passed in and out of the method and any side effects. Lots of little lego blocks seem easier to manipulate to me.

Clearly, it's easy enough to search around within the project to look for other uses. But of course, this all breaks down the minute that the class becomes public and the code gets shipped -- all of a sudden, there's an external-maintenance nightmare waiting to happen (assuming the factored-out methods are not protected or public).

I always make extracted methods private to start with. If it gets promoted to 'package private', protected or public later, well that means the method was reusable as is.

Of course if you're talking about a published interface that other teams or project use, then yes you have to be very careful not to publish methods prematurely.

From Patrick:

But IMO, separating things out into appropriate blocks of reuse is a refactoring that is often best left for when that functionality needs to be reused.

IMHO, this kind of coding is not about reuse, its about writing understandable, maintainable, extensible code.

Your example where the code blocks are delimited by comments looks quite neat now, but I see this kind of thing all the time and it quickly de-generates into long long methods with lots of local variables which are a nightmare to understand and maintain.

If you write clean code, it is more likely to stay clean as people maintain it.

If you write grubby code, it will very soon become very smelly indeed because when you're working to a deadline, most people don't have the time to take out the scrubbing brush. (sorry, got a bit carried away with my analogies)

-Martin

Your example where the code blocks are delimited by comments looks quite neat now, but I see this kind of thing all the time and it quickly de-generates into long long methods with lots of local variables which are a nightmare to understand and maintain.

Absolutely -- as I mentioned, these types of constructs often evolve into well-formed methods etc. But more often than not, these delineations are not based on what I originally anticipate. Letting methods turn into 4000-line beasts is definitely bad.

If you write clean code, it is more likely to stay clean as people maintain it.

Indeed -- the good-ole broken-window principle. However, the conjecture that breaking up a big method into smaller methods == cleaner code is one that I do not completely agree with.

-Patrick

--
Patrick Linskey
http://solarmetric.com

In practice trying to extract useful sub-methods immediately as a higher method's code is evolving can lead to many unwanted iterations of refactoring. Using blocks and then creating sub-methods only when the blocks clearly depict "separability" is a neat idea.

Wouldn't it be nice if, in the IDE, we could expand or shrink the definitions of called functions in the body of the code that calls them? Would this be a good idea? Probably should only be limited to short functions.

That might not be as useful as it sounds -- you often find when you "go-to" a function definition that you need to look at the defs of functions _it_ calls, and so on.

The traditional "go-to def" that most IDEs support works fine for this, but how would inline expand/collapse do it?

John Hurst

Good article.

I know its (very) old now but I still love:

/*
* Switch to stack of the new process and set up
* his segmentation registers.
*/
retu(rp->p_addr);
sureg();
/*
* If the new process paused because it was
* swapped out, set the stack level to the last call
* to savu(u_ssav). This means that the return
* which is executed immediately after the call to aretu
* actually returns from the last routine which did
* the savu.
*
* You are not expected to understand this.
*/
if(rp->p_flag&SSWAP) {
rp->p_flag =& ~SSWAP;
aretu(u.u_ssav);
}
/*
* The value returned here has many subtle implications.
* See the newproc comments.
*/
return(1);

From the 6th? edition unix.

I'm in total agreement with all the other points, but I'm unsure about the benefits of breaking up methods to such an extent. Unless there actually is a reason to reuse a block of code, couldn't giving it its own method give the impression that it's being reused when it isn't? Even a private method indicates to me that it's being called in two or more places within the class.

Also, couldn't breaking up a single method into lots of smaller ones cause a problem with method-level variables? Anything that's relevant to more than one method would have to be passed in as a parameter. Changes to the code could become more complicated when a load of method signatures need to be changed.

I must, however, I do like the look of the example given - very self-explanatory :)

Oh, and why does every IDE I've ever come across put crappy comments on auto-generated getters and setters by default!!?

In reply to the other Martin

Like I said, I don't see it being about re-use.

However, I understand your concerns about having to pass lots of parameters between methods as that in itself gets ugly.

But when you find yourself doing that, its an indication that those variables and the methods that operate on them have a distinct purpose of their own. So I would then break up those few methods into a new helper class with those variables as class fields rather than method parameters.

Again, this is not about re-use. Make it a private static class if you like. Its about keeping things small and manageable (oh and did I say testable, because that's one of the most important reasons for doing all of this).

After going thru the article, it seems like the author is unaware of the Javadocs comments and mixing the same with the Java comments. I know it hurts to write simple Javadocs comments for getter/setter in a bean but they are important to complete the Javadocs for the class.

Rest I completely agree with the author that Java comments should be small and sweet and not explain the obviously visible stuff from the code. I esp. don't like comments like "// end if", etc. Comments should be for a block of code just establishing what that block of code is supposed to do, or if the developer wants to leave a small instruction about the chunk of code regarding the change impact or certain not so obvious decision.

Javadocs are also very important, and a balance has to be made as in what should go as part of Javadocs and what shall go as part of Java comment.

Regarding java doc comments......

It would be useful if there was an agreed convention for 'No Further Information Necessary' (NFIN?) on methods that are self describing. IDEs like eclipse 'enforce' java doc comments, which is a good thing as it reminds the developer to consider if more information is necessary, but where the methods are self describing it is very tedious. (....'baby Jesus weeps' as one commentator put it, ROTFLMAO!)

Generally I find comments to be more useful at the class level, i.e. describing what the class is, how it should be used. 9 out of 10 times method level comments are a waste of time. Comments describing WHY code is written in a particular way (in the non-obvious cases) are worth their weight in gold.

IDEs like eclipse 'enforce' java doc comments,

Disabling that feature for methods was one of the most productive things I've done...

Generally I find comments to be more useful at the class level, i.e. describing what the class is, how it should be used.

I agree and left this feature on in Eclipse. I find that atleast a brief class level comment is always in order, to explain the abstraction. As you said, the methods should then support this abstraction, and often don't require additional comments.

... I know it hurts to write simple Javadocs comments for getter/setter in a bean but they are important to complete the Javadocs for the class.

If the comment doesn't add any information, but rather duplicates what's given in the method name - typical with getter/setter methods, how does this complete anything? The method signature is always included in the JavaDoc. I admit the additional comments "fill out" the generated JavaDoc and arguably make it look a little more polished, but I don't think it adds any practical value. If the JavaDoc will be used external to your team/company, then it may be a worth debate. But if it's strictly an internal API - the case for most projects, I think it an utter waste of time to strive for "commplete" JavaDoc.

I admit the additional comments "fill out" the generated JavaDoc and arguably make it look a little more polished, but I don't think it adds any practical value. If the JavaDoc will be used external to your team/company, then it may be a worth debate. But if it's strictly an internal API - the case for most projects, I think it an utter waste of time to strive for "commplete" JavaDoc.

I think redundancy is always bad and dangerous.

If some slave to a rigid process really requires trivial "Sets the Foo" comments just to make the javadoc "complete" and "look nice", the ideal solution would be to generate the comment automatically. Maybe the functionality could be added directly to the javadoc tool as an option.

Or maybe not, let the fools create their own tools (sorry the rude attitude, I just couldn't resist the rhyme :-)

I esp. don't like comments like "// end if", etc.

I actually like these a lot, especially when pairs of braces span over more than ten lines and nest several loops. I think they help understanding the code a lot.

I don't know about you, but a } means absolutely nothing to me since I have no idea what exactly it is we are closing.

--
Cedric

I don't know about you, but a } means absolutely nothing to me since I have no idea what exactly it is we are closing.

That's why proper indentation is important ;-)

Peace,

Cameron Purdy
Tangosol, Inc.
Coherence: Cluster your POJOs!

I know this was meant as a joke (I'm smart like that ;-) but I'd also emphasise the importance of white space in laying out code to make it more readable.

Tabs are an obvious example, but so are blank lines. The example of 'good' code given in this article actually makes me cringe, due to the way in which the code is packed into such a tight space (perhaps I suffer from latent claustrophobia?).

Maybe it's a 'C' thing to keep empty lines to a minimum - but I find it really helps me, and (imho) makes the code more pleasing from an aesthetic standpoint - which is often overlooked as a virtue.

I'm convinced that some developers have the view that 'tight' code runs faster ;-)

I'm convinced that some developers have the view that 'tight' code runs faster ;-)

As everyone knows, more lines means more bugs. So, anyone who's taken a basic class in probability and statistics will be able to see that eliminating whitespace is an easy way to reduce your bug %ages!

As an added benefit, less lines of source (and more lines of test) means a higher ratio of test to source, which again, is obviously good form. So clearly, you should be reducing empty lines in your source code, and adding them to your test code.

-Patrick

--
Patrick Linskey
http://solarmetric.com

As everyone knows, more lines means more bugs.

Aha, but more Lines Of Code per day implies greater productivity, as ani fule kno ;-)

I esp. don't like comments like "// end if", etc.

I actually like these a lot, especially when pairs of braces span over more than ten lines and nest several loops. I think they help understanding the code a lot.I don't know about you, but a } means absolutely nothing to me since I have no idea what exactly it is we are closing.-- Cedric

Hmmm. Over ten lines and nested loops? Sniff, Sniff. What is that smell? :)

Hmmm. Over ten lines and nested loops? Sniff, Sniff. What is that smell? :)

There is not necessary a smell.

I have seen very readable code span way over ten lines and include nested loops, and I've seen horrible code crammed in less lines or so spread out in sub-methods that it was much harder to read.

Besides, various code conventions (open brace on a line of its own for example) will definitely create methods that are way longer than a screenful yet surprisingly readable.

Let's stop being dogmatic here, you don't judge code by the number of lines or statements that it contains.

--
Cedric

Besides, various code conventions (open brace on a line of its own for example) will definitely create methods that are way longer than a screenful yet surprisingly readable.

There's still hope for you then, Cedric.

Peace,

Cameron Purdy
Tangosol, Inc.
Coherence: Cluster your POJOs!

Besides, various code conventions (open brace on a line of its own for example) will definitely create methods that are way longer than a screenful yet surprisingly readable.

There's still hope for you then, Cedric.Peace,Cameron PurdyTangosol, Inc.Coherence: Cluster your POJOs!

Don't you put words in my mouth, you evil formatter.

My message was one of tolerance, not recognition.

--
Ced

Don't you put words in my mouth, you evil formatter.

My message was one of tolerance, not recognition.

--
Ced

I also like how you don't mind using white-space when you post to TSS. I hope your code benefits from the same open mindset.

Correct indentation and plenty of white-space .. now all you need is hungarian notation and you'll be home. ;-)

Peace,

Cameron Purdy
Tangosol, Inc.
Coherence: Cluster your POJOs!

Hmmm. Over ten lines and nested loops? Sniff, Sniff. What is that smell? :)

There is not necessary a smell.I have seen very readable code span way over ten lines and include nested loops, and I've seen horrible code crammed in less lines or so spread out in sub-methods that it was much harder to read.

Just cause there is a smell doesn't mean there is something wrong. It might have been something in the air. :)

Greater than 10 lines and nested might be readable. But if you need comments ... .

Also smells are not just about readability. But testability, etc. (No need for me to post the Refactoring book here.)

Forget smells, and other such arm waving rubbish.

If it's an API and folks code to the api without access to source then javadoc loads - all the params, returns and funcs at the very least.

Overwise write as many comments a you would need to step back into the code at some future date - like next week :)
i.e. write comments for yourself. The person who ends up supporting your code will have his own style anyway. Don't even try to second guess him/her. And don't worry about dogma - move about enough in the industry and you come accross every kind of mutually exclusive dogma. They are not all correct, or incorrect. But the very fact they are inforced shows they are rubbish.

So write a comment containing as much information as you would need yourself to understand what/how/why.

Jonathan

So write a comment containing as much information as you would need yourself to understand what/how/why.Jonathan

IMHO ideally this is how information should be written in the source file:

how: the code
what: the method name (plus arguments and return value)
why (and other higher-level explanation): javadoc and comments

Violating the DRY principle here needs strong arguments. Most of the times programming languages are expressive enough to avoid comments on the "how" level, if the problem is divided to small enough pieces.

Of course there are exceptions, since absolute rules are always bad (except this one :-).

My opinion on minimal in-code comments.

I'm doing code review for 50+% of my time, and I encounter lots of code where comments fully are missing. A programmer offen assumes everybody else has got the same mental model of the application as he, unfortunatly others often don't.

What would help is to have at least some comments like Patrick Linskey shows in the 5th comment from this article.

public void authenticateUser() {
    // find authentication method
    {
        ...
    }

    // authenticate host if necessary
    {
        ...
    }

    // get password from the user
    {
        ...
    }

    // sound alarms as needed
    {
        ...
    }
}

Beginner programmers tell you why they are doing it, IMO just so they can understand it themselves. The very good developers don't comment at all because the code if obvious for them. Some where in the middle of a developers learning curve, just for a little while, there is a little moment they just tell you this little line what they are doing, and that's just what I as a reviewer need to understand the code.

regards Bas

So, as a reviewer, you couldn't read/understand his code example?

public void authenticateUser() {
    findAuthenticationMethod();
    if (isNetworkConnection()) authenticateHost();
    String password = getPasswordFromUser();
    if (invalidPassword(password)) soundAlarm();
}

So, as a reviewer, you couldn't read/understand his code example?

public void authenticateUser() {&nbsp;&nbsp;&nbsp;&nbsp;findAuthenticationMethod();&nbsp;&nbsp;&nbsp;&nbsp;if (isNetworkConnection()) authenticateHost();&nbsp;&nbsp;&nbsp;&nbsp;String password = getPasswordFromUser();&nbsp;&nbsp;&nbsp;&nbsp;if (invalidPassword(password)) soundAlarm();}

Yes that's perfectly clear (almost like a dream). I interpret the { ... } as beeing a block of multiple lines and the whole method would stretch 2+ screens (I know, this isn't good programming either but it happens to often).

Bas

ps. to Cameron: if a developer's quality is measured to its delivered code quality then you are absolutly right. I should have added more nuance here.

The very good developers don't comment at all because the code if obvious for them.

That is a very dangerous perception.

Very good developers comment well, and they comment neither too much nor too little.

If they do not do so, they are (by definition) not "very good developers".

Peace,

Cameron Purdy
Tangosol Coherence: Clustered Shared Memory for Java

Like 2012 wedding dress, bridesmaid attire ought to possess the needed excellent to match the profile belonging in the route of bride and her loved ones.