Re: "dselect" replacement team
jgg@gpu.srv.ualberta.ca (Jason Gunthorpe) wrote on 19.04.97 in <[🔎] Pine.A32.3.93.970419160444.51084A-100000@gpu5.srv.ualberta.ca>:
> On 19 Apr 1997, Kai Henningsen wrote:
>
> > jgg@gpu.srv.ualberta.ca (Jason Gunthorpe) wrote on 13.04.97 in
> > <[🔎] Pine.A32.3.93.970413164332.54672I-100000@gpu5.srv.ualberta.ca>:
> >
> > > Now, in my definition clear code requires a very high comment density,
> >
> > Interestingly enough, I, on the other hand, think that code with a high
> > comment density is very unclear and hard to read.
> >
> > Maybe that's because I believe that _most_ code should be written (and
> > _can_ be written) to be easily understandable without any comments at all.
> > That's what descriptive names are for.
>
> Yes, if you already know what the code should be doing, consider:
>
> Writer->CreateDataBase();
> Writer->ElementMask("Foo","Bar");
> Writer->StreamOut(ElementArray);
>
> Now, please tell me what that does :> Now you can guess to a quite high
> degree of detail, but I imagine you really don't know what ElementMask is.
Actually, I can guess that. For such a small snippet of code, it's very
understandable - and you usually don't look at three lines of code
isolated from anything else. It would certainly help to know what this is
a snippet of :-)
Now tell me what comments you would put *at this place* to make it better
understandable?
> You don't really know were this 'DataBase' is either. Now, if I only put
> comments in 'strange' places then you will have to lookup
> Writer::ElementMask, Write::CreateDataBase and Writer::StreamOut, READ
> them all and determine exactly when them an all their subfunctions do
> before you can understand what those 3 lines actuall do. This could
> easially amount to forcing you to -READ- >500 lines of code, not just
> quickly scan.
Do you mean you would explain what those members do *at this place*? Then
I certainly won't hesitate to tell you that this *seriously* degrades
readability of that code.
> I on the other hand think it's perfectly clear because I alread know what
> the database is of, what the masking function does and other details. I
Would you start explaining the file system when you do an open()? Surely
not!
The place where you call a function (or invoke a method) is never the
right place to explain what that function (or method) does. It _may_ be
the right place to explain _why_ you call it at that place.
For example, the above snippet could (depending on implementation details)
maybe use a comment (which I'd put in the line before) saying "Save all
useful elements to disk". That is, give the context.
An alternate possibility would be something like "Tricky code here. We
assume that Writer->Blurbslet is already set from the code above, which
actually inverts the meaning of the filter." That is, if this doesn't
really do what it seems to say - or if it doesn't seem to say anything
easily understandable - then a comment might be appropriate.
Of course, like I already said, it's even better to code in a style that
allows people to understand this without explicitely being told. Your
snippet seems to do well in that respect; compare to the following:
W->CDB();
W->EM("Foo","Bar");
W->SO(EA);
The difference is painfully obvious.
MfG Kai
--
TO UNSUBSCRIBE FROM THIS MAILING LIST: e-mail the word "unsubscribe" to
debian-devel-request@lists.debian.org .
Trouble? e-mail to templin@bucknell.edu .
Reply to: