[webkit-dev] Class-level comments in the source code

[Ryosuke Niwa]

On Jul 6, 2012 4:29 PM, "Maciej Stachowiak" <mjs at apple.com> wrote:
>
>
> On Jul 6, 2012, at 2:18 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:
>
>> On Fri, Jul 6, 2012 at 1:51 PM, Per Bothner <per.bothner at oracle.com
> wrote:
>>>
>>> Documenting what a function/method does is sometimes useful,
>>> but what is really important is documenting what (an instance of)
>>> a class *is* and (preferably) how it relates to other objects.
>>
>>
>> For some classes, yes.
>>
>> On Fri, Jul 6, 2012 at 2:02 PM, Maciej Stachowiak <mjs at apple.com> wrote:
>>>
>>> On Jul 6, 2012, at 12:54 PM, Per Bothner <per.bothner at oracle.com> wrote:
>>>
>>> > The biggest annoyance I found is lack of class-level comments.  For
example
>>> > what is an Interpreter?  How many instances are there in the system?
>>> > (I.e. is it a singleton class?  Is there one per window? One per
thread?)
>>> > What is the relationship to JSGlobalData, JSGlobalObject, RootObject.
>>> > There are a lot of these classes, and it takes quite a bit of staring
at
>>> > the code to figure it out. Worse, it's hard to remember it all, so if
I
>>> > come back to the codebase after working on something else I have to
>>> > figure out all out again: I might remember some aspects (like a class
>>> > starting with JS is probably some kind of JavaScript object), but not
>>> > a lot of other relationships and properties of the classes.
>>>
>>> My recollection is that in past discussions, most folks were in favor
of class-level comments that describe the purpose and nature of the class,
particularly if not self-evident from the name. I think we'd likely take
patches to take such comments. We also generally like "why" comments inside
functions - comments that explain why something is done that way.
>>
>> ...
>>>
>>> Class-level comments don't really fall into these categories, and
certainly the purpose of some of the objects you mention is not
self-evident from the name (as with e.g. AffineTransform).
>>
>>
>> I'd argue that we should still strive to make class names
self-explanatory as much as possible, and use comments as the last resort.
>>
>> I've found that our culture of not adding comments have given me a
pressure to think really hard to come up with better class names rather
than going with vague names with explanatory comments. In some cases, it
made me realize that the particular object relationships I had in my mind
was not a good design and made me come up with a new design that involved
easier-to-understand classes.
>
>
> I agree that self-explanatory class names are best. On the other hand,
sometimes it's hard to make a name that is super clear but reasonably
concise. For example, I'm not sure how I'd improve the name of JSGlobalData
to make it sufficiently clear. What's really need is a "why" explanation,
not a "what" explanation - why you need this special global data object.
That seems like a good use for a comment. On the other hand, commenting
AffineTransform with "// This object represents an affine transform in 2D
space" is probably not super helpful.

Yes, I agree. As I said on the other thread, reviewers should make a good
judgement call as to whether we should be adding a comment or not. The
comment about there's one to one mapping between threads and JSGlobalData
is helpful for example.

One big question I have is to where we should document inrer-class
relationship. g.g. I've always thought we need a some comment somewhere
explaining the differences between Range, Position, VisiblePosition,
RenderedPosition, VisibleSelection, and FrameSelection but I don't know to
where I should put it.

- Ryosuke
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20120706/5e5e1761/attachment.html>