Thunderbird:CodeCleanup

In preparation for Thunderbird and Seamonkey moving to frozen linkage/libxul/XULRunner, we need to remove our use of nsXPIDLStrings from mailnews (along with other changes). 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.

In starting to make these changes, we've noticed inefficient code patterns in our C++ code in mailnews. A lot of of these patterns can be improved as part of the AString API changes. 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

We aren't trying to replace that document, just highlighting some of the patterns we see a lot of / improvements we can make.

nsCString value;
- PL_strcase(value.get(), "imap://");
+ StringBeginsWith(someString, NS_LITERAL_CSTRING("imap://");

The following applies to both nsXPIDLString and nsXPIDLCString. This class is going away when we use frozen linkage so we want to replace it with If you are changing an interface such that somemethod now takes a nsAString / nsACString then you don't need the getter_Copies either:

- nsXPIDLString strValue;
- somemethod(getter_Copies(strValue));
+ nsString strValue;
+ somemethod(getter_Copies(strValue)); or somemethod(strValue);

nsString uniValue;
nsCString charValue;
- uniValue.Assign(NS_ConvertASCIItoUTF16(charValue));
+ CopyASCIItoUTF16(realName, fullName);

nsCString charValue;
nsString  uniValue;
- charValue.AssignWithConversion(uniValue);
+ LossyCopyUTF16toASCII(uniValue, charValue);

nsCString charStr;
- PL_strchr(uri.get(), ' ')
+ uri.FindChar(' '); (kNotFound)

nsCString charStr;
- PL_strcasecmp(charStr.get(), "nocopy://")
+ charStr.LowerCaseEqualsLiteral("nocopy://")

nsCString charStr;
- nsCRT::strcmp(charStr.get(), charStr2.get()) == 0
+ charStr.Equals(charStr2)

nsCOMPtr

Use swap to assign an object wrapped by a nsCOMPtr into a return variable. 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

nsCOMPtr<nsIMsgIdentity> identity;
- NS_IF_ADDREF(*aIdentity = identity);
+ identity.swap(*aIdentity);
return NS_OK;

- *aIdentity = m_identity;
- NS_IF_ADDREF(*aIdentity);
+ NS_IF_ADDREF(*aIdentity = m_identity);

Validating Input Arguments

NS_ENSURE_ARG_POINTER NS_ENSURE_SUCCESS

nsISupportsArray

Use do_QueryElementAt to get an element from an nsISupportsArray.

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

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

Other Things To Look For

• Remove trailing spaces
• Replace any tabs with spaces
• In most files we use a tab width of two spaces. Adjust 4 spaces tab widths.
• Some of the methods in our IDL files start with an upper case letter. When cleaning up an interface, change these to lower case. (Don't forget to search through http://mxr.mozilla.org and fix any JS callers of the API to also be lower case!)
• The more nsCRT/PL_strcmp routines that get replaced with nsString equivalents as part of the string cleanup, the easier it will be to make a later pass through the code to prepare for using frozen linkage where we won't get nsCRT routines anymore.