Last Comment Bug 722581 - Add note about stl::List to mfbt/LinkedList.h
: Add note about stl::List to mfbt/LinkedList.h
Product: Core
Classification: Components
Component: MFBT (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla13
Assigned To: Justin Lebar (not reading bugmail)
Depends on:
Blocks: 715405
  Show dependency treegraph
Reported: 2012-01-30 17:23 PST by Justin Lebar (not reading bugmail)
Modified: 2012-02-05 04:04 PST (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1 (3.69 KB, patch)
2012-02-01 10:28 PST, Justin Lebar (not reading bugmail)
jwalden+bmo: review+
Details | Diff | Splinter Review

Description User image Justin Lebar (not reading bugmail) 2012-01-30 17:23:49 PST
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?
Comment 1 User image Jeff Walden [:Waldo] (remove +bmo to email) 2012-01-30 17:50:29 PST
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 User image Justin Lebar (not reading bugmail) 2012-02-01 10:28:19 PST
Created attachment 593514 [details] [diff] [review]
Patch v1

Review: jwalden+bmo
Comment 3 User image Justin Lebar (not reading bugmail) 2012-02-01 10:32:09 PST
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.
Comment 4 User image Jeff Walden [:Waldo] (remove +bmo to email) 2012-02-03 13:35:44 PST
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.
Comment 5 User image Ed Morley [:emorley] 2012-02-05 04:04:39 PST

Note You need to log in before you can comment on or make changes to this bug.