Closed Bug 685905 Opened 8 years ago Closed 8 years ago

Add a ReplaceElementAt to nsTArray

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla9

People

(Reporter: mounir, Assigned: atulagrwl)

Details

Attachments

(1 file, 2 obsolete files)

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...
Attached patch Patch v1 (obsolete) — Splinter Review
Assignee: nobody → atulagrwl
Attachment #559664 - Flags: review?(jonas)
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+
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+
Thanks a lot Jonas. I didn't knew that. I am still learning the procedures.
Keywords: checkin-needed
Attachment #559664 - Attachment is obsolete: true
Doesn't apply to inbound, please update to tip and reattach. Thanks :-)
Status: NEW → ASSIGNED
Flags: in-testsuite-
Keywords: checkin-needed
Attached patch Patch on tip.Splinter Review
Attachment #560681 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/182d4c0c1164
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.