Closed
Bug 685905
Opened 14 years ago
Closed 14 years ago
Add a ReplaceElementAt to nsTArray
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla9
People
(Reporter: mounir, Assigned: atulagrwl)
Details
Attachments
(1 file, 2 obsolete files)
1.14 KB,
patch
|
Details | Diff | Splinter Review |
It's already pretty annoying that nsTArray and nsCOMArray do not have the same method signatures but ReplaceElementsAt when we have to replace one element is quite frustrating...
Assignee | ||
Comment 1•14 years ago
|
||
Assignee: nobody → atulagrwl
Attachment #559664 -
Flags: review?(jonas)
Updated•14 years ago
|
Attachment #559664 -
Attachment is patch: true
Comment on attachment 559664 [details] [diff] [review]
Patch v1
Review of attachment 559664 [details] [diff] [review]:
-----------------------------------------------------------------
r=me with the second version removed.
::: xpcom/glue/nsTArray.h
@@ +698,5 @@
> + // A variation on the ReplaceElementsAt method defined above.
> + template<class Item>
> + elem_type *ReplaceElementAt(index_type index, const Item* item) {
> + return ReplaceElementsAt(index, 1, item, 1);
> + }
I'd rather not have this second signature. Just seems like a source of confusion since people can both pass a pointer to a object, or the object itself.
So just keep the reference version and make people use a deref operator if all they have is a pointer.
Attachment #559664 -
Flags: review?(jonas) → review+
Assignee | ||
Comment 3•14 years ago
|
||
Assignee | ||
Updated•14 years ago
|
Attachment #560681 -
Flags: review?(jonas)
Comment on attachment 560681 [details] [diff] [review]
Removed the second function as suggested by patch
For the record, if someone says "r=me with changes X", you don't really need to re-ask for review. Unless you disagree with the suggested changes or otherwise can't make them.
Attachment #560681 -
Flags: review?(jonas) → review+
Assignee | ||
Comment 5•14 years ago
|
||
Thanks a lot Jonas. I didn't knew that. I am still learning the procedures.
Assignee | ||
Updated•14 years ago
|
Keywords: checkin-needed
Updated•14 years ago
|
Attachment #559664 -
Attachment is obsolete: true
Comment 6•14 years ago
|
||
Doesn't apply to inbound, please update to tip and reattach. Thanks :-)
Assignee | ||
Comment 7•14 years ago
|
||
Attachment #560681 -
Attachment is obsolete: true
Assignee | ||
Updated•14 years ago
|
Keywords: checkin-needed
Comment 8•14 years ago
|
||
Keywords: checkin-needed
Target Milestone: --- → mozilla9
Comment 9•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•