Closed Bug 722581 Opened 10 years ago Closed 10 years ago
Add note about stl::List to mfbt/Linked
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.)
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+
Status: NEW → RESOLVED
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.