Thunderbird:CodeCleanup

Revision as of 19:44, 3 May 2007 by Mscott (talk | contribs)

In preparation for Thunderbird using libxul, we need to remove our use of nsXPIDLStrings. While we are doing this, we're taking the opportunity to modernize some of our core interfaces to use AString and ACString instead of wstring and string.

These changes have caused us to notice a lot of bad or less efficient patterns in mailnews. By documenting these patterns and the associated fixes, it will hopefully be easier for more folks to help out in the great mailnews code cleanup of 2007!

Strings

First, familiarize yourself with: http://developer.mozilla.org/en/docs/XPCOM:Strings#Common_Patterns

Bad:

 nsCString someString
 PL_strcase(someString.get(), "imap://")

Good:

 StringBeginsWith(someString, NS_LITERAL_CSTRING("imap://");

Bad:

 nsString fullName;
 nsCString realName;
 fullName.Assign(NS_ConvertASCIItoUTF16(realName));

Good:

 CopyASCIItoUTF16(realName, fullName);

Bad:

 nsCString uri;
 PL_strchr(uri.get(), ' ')

Good:

 uri.FindChar(' '); (kNotFound)

Bad:

 nsCString uri;
 PL_strcasecmp(uri.get(), "nocopy://")

Good:

 uri.LowerCaseEqualsLiteral("nocopy://")
 

nsCOMPtr

Use swap to assign an object wrapped by a nsCOMPtr into a return variable

Bad:

 nsCOMPtr<nsIMsgIdentity> identity;
 NS_IF_ADDREF(*aIdentity= identity);
 return NS_OK;

Good:

 identity.swap(*aIdentity)
 return NS_OK;

This saves the cost of a reference count. Don't do this when the nsCOMPtr object is a member variable of a class or you want to use

Validating Input Arguments

NS_ENSURE_ARG_POINTER NS_ENSURE_SUCCESS

nsISupportsArray

Use do_QueryElementAt to get an element from an nsISupportsArray.

Bad:

 nsCOMPtr<nsISupportsArray> identityArray;
 nsCOMPtr<nsIMsgIdentity> identity;
 rv = identityArray->QueryElementAt(index, NS_GET_IID(nsIMsgIdentity),
                             (void **)getter_AddRefs(identity));

or

 nsCOMPtr<nsISupports> thisElement;
 identityArray->GetElementAt(index, getter_AddRefs(thisElement));
 nsCOMPtr<nsIMsgIdentity> identity = do_QueryInterface(thisElement, &rv);

Good:

 nsCOMPtr<nsIMsgIdentity> identity( do_QueryElementAt(identityArray, i, &rv));