Closed Bug 792169 Opened 8 years ago Closed 8 years ago
_New IMutable Array
In the interest of dispensing with nsISupportsArray, it would be nice if nsIMutableArray had a corresponding constructor function to NS_NewISupportsArray. Granted, there's nsArrayConstructor, but that's internal to nsArray and the xpcom module, and requires mucking about with IIDs and whatnot.
Why not just "new nsArray"?
(In reply to Benjamin Smedberg [:bsmedberg] from comment #2) > Why not just "new nsArray"? nsArray.h is not exported. nsArrayConstructor also contains a bit of logic to create cycle-collectable arrays if we're running on the main thread. It seemed unwise to require people to know which kind of array they wanted.
Hrm. I'd really like this to be a C++-y as possible, not perpetuating the old and generally bad NS_New structures. Why not do this: * make the nsArray constructor protected * add a `public: static already_addRefed<nsIArray> Create()` and a comment about how it will create a cycle-collectable array on the main thread and a non-CC array on other threads
Comment on attachment 662270 [details] [diff] [review] patch OK, I can try to do something like that. I thought the whole point of the NS_New* functions was to avoid exporting the internals of the implementation and/or providing more efficient/direct ways to create things without having to go through do_CreateInstance. I guess that's not the case?
Avoiding do_CreateInstance, yes (although unless it's in tight loops it doesn't show up in profiles, it's just ugly). Avoiding exporting may have been a goal in the past, but I don't think it's really worthwhile. Let's embrace our C++-ness.
In the interest of safety, I made the constructors non-public and moved the XPCOM constructor function into nsArray itself. I also cleaned up the comment block before nsArray; it doesn't look like any of it is applicable anymore. The one thing I'm not sure about is losing the null check in nsArray::Create as opposed to nsArray::XPCOMConstructor. But ISTR that new is infallible, or has become so lately...?
Comment on attachment 663430 [details] [diff] [review] revised patch Yes, new is infallible now so you can safely remove the null-check.
Attachment #663430 - Flags: review?(benjamin) → review+
Given comment 8, I took the liberty of removing the null check from XPCOMConstructor, too. https://hg.mozilla.org/integration/mozilla-inbound/rev/711db2ce69f0
Assignee: nobody → nfroyd
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.