Repetitive XML comments -- what's the point?

[Dave Sexton]

Hi Mark,

No need to throw us a bone -- we're doing just fine

 

[Willem van Rumpt]

Not commenting code is a very good way of keeping "ownership" of code.
I once worked for a company who thought they had bought the source
code to a large project. Well, they had, I suppose. Unfortunately, one
of the programmers for the selling company thought that the code was
"his", so he removed all the comments, thus rendering it pretty much
useless. I found, as the new maintainer of the code, that it was very
hard to keep the project going, even though about 40% of the code had
actually been written by me several years previously. Good, relevant
comments are invaluable.

If the algorithm is complex enough: Sure. There are cases when I feel,
though not often, that refactoring an algorithm makes an algorithm
harder to grasp than when it would've been coded as a single method.

From my experience, the quality of comments inside method bodies tend
to go downhill to the level of "declare i and initialize to zero", "loop
through the array and process every element", instead of actual usefull
information. That sort of information clutters the code, and I don't
want it: The code itself explains that.

When I feel the need arise to document the source itself, my first
reaction is to see if the code itself can't be made more self
explanatory. In most cases, it can.

 

[Laurent Bugnion]

Hi,

I work on a team where comments are forbidden (except for exceptional
conditions). And yet we deliver quality software on time.

Rereading my reply, I see that it sounds as a criticism, which was not
what I wanted. Sorry about that. I also saw in another post that you use
agile processes. I don't have much experience with that. I guess that
this would explain the "disagreement".

How could this be?

1) We write good code (that's what I've been saying throughout this thread).

I deliver good code, on time, and it's documented ;-) Granted, I work in
a very traditional firm, where agile development is not widespread. That
said, I like to have the time to document my code. I find that it helps
me to improve it. It's the same train of thoughts that leads me to blog
about my code when I find it interesting enough, I just happen to like
re-analyzing what I did, and understanding it better.

That said, many developers are utterly unable to write good comments, I
will agree to that!

2) We all work on all the code. When it comes time to divide up tasks,
preference is given to working on code you haven't worked on before. We
don't have the concept of "taking over his code." We often work on code
together. We do most design in common.

Works for us!

///ark

That's an interesting way. Thanks for sharing.

Greetings,
Laurent

 

[Laurent Bugnion]

Hi,

Rereading my reply, I see that it sounds as a criticism, which was not
what I wanted. Sorry about that.

Greetings,
Laurent

 

[Ben Newsam]

When I feel the need arise to document the source itself, my first
reaction is to see if the code itself can't be made more self
explanatory. In most cases, it can.

People who write the code should not be the ones who decide whether or
not it is self-explanatory. If someone asks "What does this bit do?",
that's the time to add comments. If it does what it is meant to, there
is no need at that stage to change the code itself.

 

[Mark Wilden]

// Up the left side and across the top,
// two pixels away from the window.
hPen = CreatePen ( PS_SOLID, 0, bRaised ? WHITE : BLACK );
hOldPen = SelectObject ( hDC, hPen );
ptPoints[0].x = rcRect.left - 1;
ptPoints[0].y = rcRect.bottom;
ptPoints[1].x = rcRect.left - 1;
ptPoints[1].y = rcRect.top - 1;
ptPoints[2].x = rcRect.right;
ptPoints[2].y = rcRect.top - 1;
Polyline ( hDC, ptPoints, 3 );
SelectObject( hDC, hOldPen );
DeleteObject( hPen );

I think I would refactor that code. Note that you're using rcRect.left - 1
twice and rcRect.top - 1 twice. If you chose to make it three pixels away,
you'd have to change the code in a number of places. I'm not sure
exactly -how- I'd refactor it without knowing the greater context, but
duplication is always a "smell," the removal of which can often make code
more readable.

I am certainly not going to make any of the variables more meaningful than
they already are.

One thing I find detracts from its readability is the hungarian notation.
There's extra characters there with no extra meaning.

If this were C++, I would use the RAII idiom to compress the four lines
dealing with pens.

However, I do grant you that graphical manipulation (which I'm not an expert
on by any means) does lead to more complicated code. And complicated code
should indeed be commented.

///ark

 

[Mark Wilden]

Not commenting code is a very good way of keeping "ownership" of code.

See my other comment about how, at my shop, we forbid both commenting and
ownership. Turns out the practices work well together.

Unfortunately, one
of the programmers for the selling company thought that the code was
"his", so he removed all the comments, thus rendering it pretty much
useless.

How could it be useless, when the code itself can be read? Was it really
written that badly?

///ark

 

[Mark Wilden]

Yes, it most certainly does. But "taking into consideration" doesn't mean
that it's ok to support the development of code without documentation.

If "ok" means, "possible to ship quality product on schedule," then, yes, it
does (at least, at my shop).

It is a skill just like coding is a skill.

Of course you're right. The question is whether developers should spend
their finite time documenting or programming.

They specify what's assumed to be correct at the time they pass.

Sure, just as the documentation (in theory) specifies what's correct at the
time it's written. The big difference is that the tests -prove- something.

The code is never guaranteed to be right, however.

It is guaranteed to be what you ship. It is guaranteed to be what your
customers pay for. It is guaranteed to be what your developers produce. None
of which can by claimed by documentation.

Do you trust the people on your team to write good code?

No. "Trust" has no place in technology. That's why we do frequent
pair-programming and write lots of tests. From what I've seen, the people on
my team -do- write good code and that's my working hypothesis, but I don't
"trust" that they do.

You're really fixated on the idea that you can't trust documentation.

Well, you've admitted yourself that documentation can't be trusted. You go
on to say that we should try to make it more trustworthy, but no one here is
saying that the current state of play is that documentation is worth
trusting.

But then again, agile guys don't write buggy code I guess

We release less buggy code, because we unit-test everything we write.

I'm not sure we're ever going to agree on this.

That's not the goal.

Man, I wish we could agree on something! Don't you just feel like we're
both just spinning our wheels?

No, we're expressing our ideas. Jump off anytime you want.

 

[Noah Sham]

1 and 3 also smell like magic numbers that should be named constant. Also,
where are the two pixels referenced in the comment ? :')

// Up the left side and across the top,
// two pixels away from the window.
hPen = CreatePen ( PS_SOLID, 0, bRaised ? WHITE : BLACK );
hOldPen = SelectObject ( hDC, hPen );
ptPoints[0].x = rcRect.left - 1;
ptPoints[0].y = rcRect.bottom;
ptPoints[1].x = rcRect.left - 1;
ptPoints[1].y = rcRect.top - 1;
ptPoints[2].x = rcRect.right;
ptPoints[2].y = rcRect.top - 1;
Polyline ( hDC, ptPoints, 3 );
SelectObject( hDC, hOldPen );
DeleteObject( hPen );

I think I would refactor that code. Note that you're using rcRect.left - 1
twice and rcRect.top - 1 twice. If you chose to make it three pixels away,
you'd have to change the code in a number of places. I'm not sure
exactly -how- I'd refactor it without knowing the greater context, but
duplication is always a "smell," the removal of which can often make code
more readable.

I am certainly not going to make any of the variables more meaningful
than they already are.

One thing I find detracts from its readability is the hungarian notation.
There's extra characters there with no extra meaning.

If this were C++, I would use the RAII idiom to compress the four lines
dealing with pens.

However, I do grant you that graphical manipulation (which I'm not an
expert on by any means) does lead to more complicated code. And
complicated code should indeed be commented.

///ark

 

[Mark Wilden]

I like to have the time to document my code. I find that it helps me to
improve it. It's the same train of thoughts that leads me to blog about my
code when I find it interesting enough, I just happen to like re-analyzing
what I did, and understanding it better.

That's sounds similar to what I and others do with "refactoring," where we
don't just bang out code and move on, but try to think about it and improve
it after the fact.

And I do agree that writing about code (i.e. documenting it) can lead to
insights. This is similar to coding about code, which is what TDD and
unit-testing is about.

///ark

 

[Mark Wilden]

1 and 3 also smell like magic numbers that should be named constant. Also,
where are the two pixels referenced in the comment ? :')

I would say that 1 should be a parameter to a function/method and the name
of that function/method would do a lot to obviate the comment. 3 should be
replaced by (sizeof ptPoints / sizeof *ptPoints). As for the two pixels, yer
right - that comment may be correct, but it's a little confusing (maybe the
line is two pixels from the interior of rcRect?).

It takes a brave man to post code here.

// Up the left side and across the top,
// two pixels away from the window.
hPen = CreatePen ( PS_SOLID, 0, bRaised ? WHITE : BLACK );
hOldPen = SelectObject ( hDC, hPen );
ptPoints[0].x = rcRect.left - 1;
ptPoints[0].y = rcRect.bottom;
ptPoints[1].x = rcRect.left - 1;
ptPoints[1].y = rcRect.top - 1;
ptPoints[2].x = rcRect.right;
ptPoints[2].y = rcRect.top - 1;
Polyline ( hDC, ptPoints, 3 );
SelectObject( hDC, hOldPen );
DeleteObject( hPen );

 

[Dave Sexton]

Hi Mark,

You've grabbed single lines out of my response and responded to them and it's not the first time. I hope you realize that you've
responded in a partial-context and that you're making it difficult for me to express my ideas to you when you're not addressing all
of them. I keep writing the same things over and over again and because of this and I see that you are doing the same, so I was
hoping that we could come to some agreement and say, for instance, that whatever works for each of us, individually, is the right
choice.

Apparently you'd rather just debate this topic until your head falls off, but we're never going to be finished "expressing our
ideas" unless we start agreeing on some things. And since that's never going to happen without analyzing the root of our debate,
which is our differences in process guidance, then we're going to just keep spinning our wheels. I'm not sure this is the
appropriate thread to do that, however, since it really is only about documentation (at the highest-level), but it's definitely very
important at a minimum to acknowledge that our experience might very well be different and that if we keep discussing this topic
using statements such as, "in my shop", "in my experience", "I've seen", "usually", "sometimes", "often" without definite context
and statistics then our differences in experience are just going to dictate whether we find documentation useful and we're never
going to agree one way or the other.

I think we have both made our points clear and I'm not sure there's anything more to say other than I find custom documentation to
be useful and that you don't, largely due to trust and time issues, and both of us acknowledge some exceptions to these beliefs.

If you're curious as to the root of the choices I make with regards to documentation in my code then you can read about the MSF
(Microsoft Solution Framework), which is a process guidance based largely on scientific principles and consulting experience.
Although the MSF might not directly address the level of xml commenting that I prefer, it's those governing principals that I use
when making the choice to document my code and it's the reason why I hope others do the same if I have to work on their code.

 

[Bruce Wood]

Ben,

Your posts are appearing as one day into the future, which means that
topics to which you respond are staying at the top of the heap in my
newsreader.

Please fix your system clock....

// Up the left side and across the top,
// two pixels away from the window.
hPen = CreatePen ( PS_SOLID, 0, bRaised ? WHITE : BLACK );
hOldPen = SelectObject ( hDC, hPen );
ptPoints[0].x = rcRect.left - 1;
ptPoints[0].y = rcRect.bottom;
ptPoints[1].x = rcRect.left - 1;
ptPoints[1].y = rcRect.top - 1;
ptPoints[2].x = rcRect.right;
ptPoints[2].y = rcRect.top - 1;
Polyline ( hDC, ptPoints, 3 );
SelectObject( hDC, hOldPen );
DeleteObject( hPen );

I think I would refactor that code.

So would I. Er... probably. Possibly. I wasn't illustrating my point
with a piece of perfect code, but with some *real* code that will
definitely *not* be altered without a really good reason (such as it
not working any more).The point is that the comments make it a whole
lot easier to understand. The code itself simply is what it is, it
does its job (whatever that happens to be) and therefore does not need
to be fiddled with in any way. Whatever circumstances that code was
written in, and whatever language it is written in, are irrelevant.
Yes, it can be improved, but it won't be, and when I found that
snippet I was grateful for the comments included.



Note that you're using rcRect.left - 1

twice and rcRect.top - 1 twice. If you chose to make it three pixels away,
you'd have to change the code in a number of places. I'm not sure
exactly -how- I'd refactor it without knowing the greater context, but
duplication is always a "smell," the removal of which can often make code
more readable.


One thing I find detracts from its readability is the hungarian notation.
There's extra characters there with no extra meaning.

If this were C++, I would use the RAII idiom to compress the four lines
dealing with pens.

However, I do grant you that graphical manipulation (which I'm not an expert
on by any means) does lead to more complicated code. And complicated code
should indeed be commented.

///ark

 

[Ben Newsam]

Ben,

Your posts are appearing as one day into the future, which means that
topics to which you respond are staying at the top of the heap in my
newsreader.

Please fix your system clock....

Oooh yes! Blimey. Thanks.

 

[Mark Wilden]

So would I. Er... probably. Possibly. I wasn't illustrating my point
with a piece of perfect code, but with some *real* code that will
definitely *not* be altered without a really good reason (such as it
not working any more).

One of the tenets of "agile" is that working code should indeed be
refactored to make it more readable and easier to use. In this case, I think
that refactoring would make the code more reusable as well, which would have
payoffs down the road (though agile tends not to value "down the road" very
highly).

One of the points we have been making is that the time taken to write a
comment can be spent making the code itself self-commenting. This has the
benefits mentioned above of well-factored code, plus it avoids the (almost
inevitable) drawbacks of the comments falling out of sync with the code.
Well-factored code is always correct with respect to itself. Comments may or
may not be, and it take additional developer cycles to determine such
correctness. Even if you were to read your comment years later, I think you
would probably still verify that the code still matched the comment,
wouldn't you?

The code itself simply is what it is, it
does its job (whatever that happens to be) and therefore does not need
to be fiddled with in any way.

What we're saying is that if you "fiddle" with the code a bit, you don't
need to comment it. And you end up with better code, to boot. That's sounds
like a win to me.

///ark

 

[Ben Newsam]

See my other comment about how, at my shop, we forbid both commenting and
ownership. Turns out the practices work well together.


How could it be useless, when the code itself can be read? Was it really
written that badly?

It wasn't brilliant, no. Pretty hard to read. That's the real world
for you.

 

[Ben Newsam]

What we're saying is that if you "fiddle" with the code a bit, you don't
need to comment it. And you end up with better code, to boot. That's sounds
like a win to me.

I have learnt, over many, many years experience, not to "fiddle" with
code at all unless it is necessary. What seems like an improvement can
have all sort of repercussions elsewhere.

 

[Mark Wilden]

I have learnt, over many, many years experience, not to "fiddle" with
code at all unless it is necessary. What seems like an improvement can
have all sort of repercussions elsewhere.

It's true that ruthless refactoring isn't practical without unit tests.

///ark

 

[Ben Newsam]

// Up the left side and across the top,
// two pixels away from the window.
hPen = CreatePen ( PS_SOLID, 0, bRaised ? WHITE : BLACK );
hOldPen = SelectObject ( hDC, hPen );
ptPoints[0].x = rcRect.left - 1;
ptPoints[0].y = rcRect.bottom;
ptPoints[1].x = rcRect.left - 1;
ptPoints[1].y = rcRect.top - 1;
ptPoints[2].x = rcRect.right;
ptPoints[2].y = rcRect.top - 1;
Polyline ( hDC, ptPoints, 3 );
SelectObject( hDC, hOldPen );
DeleteObject( hPen );

I think I would refactor that code.

So would I. Er... probably. Possibly. I wasn't illustrating my point
with a piece of perfect code, but with some *real* code that will
definitely *not* be altered without a really good reason (such as it
not working any more).The point is that the comments make it a whole
lot easier to understand. The code itself simply is what it is, it
does its job (whatever that happens to be) and therefore does not need
to be fiddled with in any way. Whatever circumstances that code was
written in, and whatever language it is written in, are irrelevant.
Yes, it can be improved, but it won't be, and when I found that
snippet I was grateful for the comments included.



Note that you're using rcRect.left - 1