Mark Banner (:standard8) Assignee
	Description • 17 years ago

There's a few places in mailnews that we create an array, and then use NS_NewArrayEnumerator to return an nsISimpleEnumerator. Given we have NS_NewEmptyEnumerator, we should be using that.

I've also included a few bits of simpler tidy up, that drop the obsolete nsISupportsArray and use nsI(Mutable)Array instead, picking up the appropriate enumerator at the same time.

	David :Bienvenu
	Comment 1 • 17 years ago

Comment on attachment 290898 [details] [diff] [review]
The fix

thx, Mark

	Mark Banner (:standard8) Assignee
	Comment 2 • 17 years ago

Patch checked in -> fixed.

	dwitte@gmail.com
	Comment 3 • 17 years ago

Comment on attachment 290898 [details] [diff] [review]
The fix

>Index: mailnews/addrbook/src/nsAbLDAPDirectory.cpp
> PR_STATIC_CALLBACK(PRBool) enumerateSearchCache(nsHashKey *aKey, void *aData, void* closure)
> {
>-        nsISupportsArray* array = (nsISupportsArray* )closure;
>-  nsIAbCard* card = (nsIAbCard* )aData;
>+  nsIMutableArray* array = static_cast<nsIMutableArray*>(closure);
>+  nsIAbCard* card = static_cast<nsIAbCard*>(aData);
> 
>-  array->AppendElement (card);
>-        return PR_TRUE;
>+  array->AppendElement(card, PR_FALSE);
>+  return PR_TRUE;

in future, you might want to consider using nsCOMArray<nsIAbCard> instead of nsIMutableArray here - it's a concrete class and the AppendElement call won't be virtual (thus faster). NS_NewArrayEnumerator works identically with nsCOMArray, as well.

	Mark Banner (:standard8) Assignee
	Comment 4 • 17 years ago

(In reply to comment #3)
> in future, you might want to consider using nsCOMArray<nsIAbCard> instead of
> nsIMutableArray here - it's a concrete class and the AppendElement call won't
> be virtual (thus faster). NS_NewArrayEnumerator works identically with
> nsCOMArray, as well.

I did consider using nsCOMArray in some of these instances. However I was dubious at the advantages considering that NS_NewArrayEnumerator has to iterate through the nsCOMArray on creating the Enumerator, whereas the nsI(Mutable)Array doesn't. I suppose GetNext doesn't have to do a QI which may help the nsCOMArray case.