Closed Bug 722581 Opened 10 years ago Closed 10 years ago

Add note about stl::List to mfbt/LinkedList.h


(Core :: MFBT, defect)

Not set





(Reporter: justin.lebar+bug, Assigned: justin.lebar+bug)




(1 file)

I wrote to dev.planning:

"Q: Why not use stl::List?

"A: Constant-time removal of an element from the list is, in my view,
the killer feature of doubly-linked lists.  (If you can manage without
this, you can often get away with a singly-linked list.)  With
stl::List, the prev/next pointers are not stored in the object itself,
so you can't cast an object into something which you can call remove()
on.  (Unless you store the object's iterator in the object itself, I
suppose.  But that's yucky.)

"With mozilla::LinkedList, you can call remove() directly on your
object, because your object inherits from LinkedListElement."

jrmuizel suggested I add a comment to this effect to mozilla/LinkedList.h, and waldo agreed.

Waldo, do you think this should go in the comment at the very top of the file, or in the implementation notes comment further down?
Assignee: nobody → justin.lebar+bug
I'd say in the implementation decisions.  Readers looking to understand the interface usually aren't going to care about the tradeoffs encountered.

(For what it's worth, I watch the component, so no especial need to CC me on bugs here.)
Attached patch Patch v1Splinter Review
Review: jwalden+bmo
Comment on attachment 593514 [details] [diff] [review]
Patch v1

I really have no idea why git bz attach sometimes sets the review flag and sometimes doesn't.
Attachment #593514 - Flags: review?(jwalden+bmo)
Comment on attachment 593514 [details] [diff] [review]
Patch v1

Review of attachment 593514 [details] [diff] [review]:

::: mfbt/LinkedList.h
@@ +119,5 @@
> +     *
> +     *
> +     * FAQ: Why not use stl::List instead of reinventing the wheel?
> +     *
> +     * Answser: The key difference between mozilla::LinkedList and stl::List is

stl::list surely, throughout?

"Answser", too, but at a lower level, I don't think this really needs the "FAQ: " or "Answer: " preliminaries.  Just stick to plain old prose, I'd say.
Attachment #593514 - Flags: review?(jwalden+bmo) → review+
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.