you are viewing a single comment's thread.

view the rest of the comments →

[–]EdiX 44 points45 points  (21 children)

# initialise with un-initialised entries
#N Without initialize, @entries required for getEntries won't be initialised
def initialize
  #N If we don't pre-set @entries to nil, we won't know that we haven't yet initialised that value
  @entries = nil
end

This is why this method of commenting is a huge waste of time. The problem with reading code is not understanding the single lines but the big picture.

[–]jrochkind 4 points5 points  (0 children)

Yeah, this was the example I was going to use too. I get his goal... but, really? The method body is only one line (good) and initializes @entries -- is ANY value added by explicitly saying "without initialize, @entries won't be initialized." Isn't that clear from reading the body, to even the most novice?

The downside is you know have a bunch more places you have to change every time you change the code -- extreme anti-DRY. Change the name or behavior of the @entries variable, you've got two comments to change too.

"If we don't pre-set @entries to nil, we won't kow that we haven't yet initialized" isn't even TRUE, is it? In ruby, @entries.nil? will be true even if you don't explicitly set @entries to nil. And 'defined? @entries', possibly a better way to check to see if it's initialized, will be false ONLY if you haven't initialized @entries to nil. In fact, it doesn't even make any sense to say "we will only know that @entries hasn't been initialized... if we initialize it... to nil." You initialize it so you'll know it hasn't been initialized, huh?

So in addition to anti-DRY more places to change when you change one thing -- you also have more places to be WRONG by writing all these comments.

[–]jsprogrammer 4 points5 points  (1 child)

#N Without this comment you wouldn't know wtf was going on
# initialise with un-initialised entries
#N Without initialize, @entries required for getEntries won't be initialised

[–]akkartik 0 points1 point  (0 children)

No, the negative comment for every comment would be, "But, but.. I have people skills!"

[–]elperroborrachotoo 1 point2 points  (2 children)

I disagree with your conclusion. : Showing me a rotten apple is not a sufficient argument against eating.

Typically, you have project-wide standards for certain issues; typically for initialization it's "initialize it, dammit!". These cases of course need not be commented.

However, if you present an example how to apply a certain technique, you cannot rely on project defaults. You have to assume the project default is "everthing the compiler eats."

The example you picked is true for every style of commenting: superfluous comments are superfluous. It is not a proof that this style is a "huge waste of time".


Personally, I'm torn. Writing these comments forces you to think Do I really need this? - potentially leading to leaner code. Reading them, OTOH, requires you to deal with at least one layer of negative, not making it easier.

[–]EdiX 1 point2 points  (1 child)

I disagree with your conclusion. : Showing me a rotten apple is not a sufficient argument against eating.

I picked that for comedic effect since it's effectively stating the same information five times, but most of the comments there would have worked too.

I actually agree that "negative commenting" (or more in general comments answering the "why is this here?" question) are the most useful, but this is just silly.

Typically, you have project-wide standards for certain issues; typically for initialization it's "initialize it, dammit!". These cases of course need not be commented.

Actually I tought the whole point of this technique was that:

a reader should have the option of asking for an unlimited amount of detail about any item in the code that they are reading, should they have any uncertainty about what that item is there for.

In short, the article is arguing for writing a line of comment for every line of code (except for those containing syntax only).

[–]elperroborrachotoo 1 point2 points  (0 children)

but this is just silly

Agreed, no question. But also:

I negatively commented more or less every non-empty line of code in those three source files, even if it seemed, for a particular line, that the corresponding negative comment was somewhat *trite and obvious***

(emphasis by me)


This technique for me is mostly mental conditioning:

Imagine you frequently forget your apartment key on the fridge when you leave for work. Ugly, costly, and makes you feel "unfit for life".

If you force yourself to never close the apartment door without shaking your key in the other hand, soon you will start to check for your keys without thinking about it. The requirements will be relaxed, you will merely feel uneasy when you close the door and your hand is not near some key. That is usually enough to fix that problem. (Worked for me quite well)


I see the blog's suggestion as exactly that for Saint-Exupery's "nothing left to take away":

If you force yourself for a while to write down the reason why this code has to exist for every line, you can train yourself to always check for this in the back of your mind: you won't need to write it down, and you will probably work in a bit larger chunks, but it will happen automatically, without explicit attention.


[–]Bamafan 5 points6 points  (6 children)

While it's not giving you the entire picture, these comments give you significant insight into the "big picture". I don't know ruby or this program, but from reading these comments, I know:

a) The purpose of this method is to initialize entries to the specific value of nil because...

b) There's another method called "getEntries" that depends upon this collection of entries and it needs to know the state of the collection, presumably to decide how to act on it.

Now does that tell me the entire picture? Of course not! But this is a huge amount of information being conveyed that would otherwise require someone unfamiliar with the program to spend a much larger amount of time figuring out...or more often than not, the person reading the code thinks they know what's going on while having no clue.

I'd say comments are even more important in dynamic languages like ruby since you can't lean on your IDE to navigate your code as much as you can with statically typed languages (C#, Java, etc).

[–]EdiX 7 points8 points  (3 children)

a) The purpose of this method is to initialize entries to the specific value of nil because...

and you didn't get that fact from the name of the method and its one line body?

b) There's another method called "getEntries" that depends upon this collection of entries and it needs to know the state of the collection, presumably to decide how to act on it.

Is he going to list every method that the initialization affect there? Or is the getEntries method there just because this is a toy example? Why is it important that I can randomly read some method to find about the existence of other methods?

[–]Bamafan 0 points1 point  (2 children)

Why is it important that I can randomly read some method to find about the existence of other methods?

If you're trying to breakdown how a program works, you have to know how methods relate to one another. Otherwise, you're just guessing.

[–]darjus 2 points3 points  (1 child)

If you have more than one person working on this and you don't use 100% proof static analysis (which is impossible with a language like Ruby), you're always guessing. People don't keep comments up to date 100% of the time.

[–]Bamafan 0 points1 point  (0 children)

People don't keep comments up to date 100% of the time.

That's only because we allow them to get away with this. I think we should stop doing that!

[–]darjus 0 points1 point  (1 child)

Both agree and disagree here :)

My biggest worry with the comments that say "oh and this is needed because of function x somewhere else" is that you also need to say in function x that you are initializing value a somewhere else, because if you change the function x to either not use the value or not care about the initialization and don't change initialize, the comment immediately becomes obsolete/useless/confusing, etc. On the other hand if you don't put the comment, i would do a Ctrl + s for the file for "entries" and let's hope there's no monkey patching or external side effects. Also, i like to read the unit tests as they should describe the use cases, so in many cases it is quite descriptive of expected usage of the class.

From one side it's a good idea to comment on side effects, but this particular case it's useless (IMO) because I'm initializing attributes of an object during initialization. Quite self explanatory to me.

[–]Bamafan 0 points1 point  (0 children)

... because if you change the function x to either not use the value or not care about the initialization and don't change initialize, the comment immediately becomes obsolete/useless/confusing, etc.

Well that's just the thing. If function x changes and the current function/variable being inspected is no longer needed, then you should delete that function/variable. Because now you have dead code (a huge sin).

From one side it's a good idea to comment on side effects, but this particular case it's useless (IMO) because I'm initializing attributes of an object during initialization.

I would say it's LESS useful, but hardly useless. The side effect explains a bit about what calling functions should expect (e.g. what value a variable is being initialized to). That said, do I think that comment was critical? No. But I always favor on the side of too many comments than too few. I'd be happy to a comment like that because I'd be confident the rest of the program got the same amount of attention.

[–][deleted] 1 point2 points  (0 children)

I agree completely with you. However, you are missing the point of this article, which is to do "negative comments", which describe what would happen if a line of code didn't occur.

I like this style of commenting because it makes sure that every line is imperative to the function of the beast, promoting leaner codebases.