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

RESOLVED FIXED in mozilla13



6 years ago
6 years ago


(Reporter: Justin Lebar (not reading bugmail), Assigned: Justin Lebar (not reading bugmail))



Firefox Tracking Flags

(Not tracked)



(1 attachment)



6 years ago
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?


6 years ago
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 2

6 years ago
Created attachment 593514 [details] [diff] [review]
Patch v1

Review: jwalden+bmo

Comment 3

6 years ago
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+
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.