[haiku-doc] Re: Phase III .diff List.dox

  • From: "Niels Reedijk" <niels.reedijk@xxxxxxxxx>
  • To: haiku-doc@xxxxxxxxxxxxx
  • Date: Tue, 22 May 2007 18:21:59 +0200

Hi Thom,

2007/5/22, Thom Holwerda <slakje@xxxxxxxxxxx>:
Comments greatly appreciated!

The patch applies cleanly, so that's a good start. Some notes:

-  have to be copied into a new piece of allocated memory which, in turn, is an
+  have to be copied into a new piece of allocated memory, which, in turn, is an

Are the extra commas around between 'in turn' necessary? It might be
correct, but it looks a bit awkward (3 comma's in a row). Plus, in
regular speech you would not pronounce it as which (pause) in turn
(high pitch ending, pause), because it not realy is in turn anyway. I
actually would like to remove the in turn at all since the allocation
is expensive, and it's not hte expensiveness that is a result of the
fact that you allocate new memory. (does that make sense?)

-  If you are unsure, you don't have to overthink this. Just make sure you don't
-  use a lot of lists and as long as the list isn't used in one of the
+  If you are unsure, you do not have to over-think this. Just make
sure you do not use a lot of lists, and as long as the list is not
used in one of the

I guess overthinking is from my original version, and I'm affraid that
it's a dutchism, since the dictionary does not mention you can think
over something, let alone overthink something. Rather change it to
'not to worry too much'.

/*!
-  \name Adding and removing items
+  \name Adding and removing items.
*/

I think both the original and the new version are wrong. This is a
header, as such it should be 'Adding and Removing Items'. I bet the
other headers share the same problem.

-  \param index The offset in the list where to put the item.
+  \param index The offset in the list where the item should be put.

Though your improvement may be more correct, it sounds a bit awkward.
Should we replace it with 'placed' or 'end up'?

-  \warning If there is anything you want, for which you need the list of
-    objects, please understand that that probably means that what you
want to do
-    is a bad idea to begin with and you should avoid this method. The list of
-    objects doesn't belong to you. See if DoForEach() can help you out instead.
+  \warning If there is anything you want, for which you need the list
of objects, please realize that that probably means that what you want
to do is a bad idea to begin with and that you should avoid this
method. The list of objects does not belong to you. See if DoForEach()
can help you out instead.

The wrapping did not go very well there. Your editor ate up all the
newlines and now it's one big line. I think there is another case
where I suddenly saw some lines disappear. Could you please
doublecheck this again.

If you fix the problem of the eaten newlines and think about my
commentary (which you are free to accept or reject), I will accept the
next patch, though probably tomorrow, since I'll be going home in a
few minutes.

Niels

Other related posts: