Joey Minta Assignee
	Description • 16 years ago

This is me trying to force myself to pick up a bit more c++. This switches nsIMsgCompField's attachmentsArray property to be an nsISimpleEnumerator, instead of an nsISupportsArray. It also switches the internal m_attachments to be a nsCOMArray.

	neil@parkwaycc.co.uk
	Comment 1 • 16 years ago

Comment on attachment 337329 [details] [diff] [review]
patch v1

>+      var attachments = msgCompFields.attachments;
>+      var hasAttachments = false;
You could write var hasAttachments = attachments.hasMoreElements();
Or you could write
if (attachments.hasMoreElements()){ChangeAttachmentBucketVisibility(false);}

>-  return m_attachments->InsertElementAt(attachment, attachmentCount);
>+  return m_attachments.AppendObject(attachment);
InsertElementAt returns an nsresult. AppendObject returns a PRBool.

>+  for (PRInt32 i = 0; i < attachmentCount; i ++)
>+    m_attachments.RemoveObjectAt(0);
Strangely nobody bothered to use the Clear() method. Now's your big chance!

>-  nsCOMPtr<nsISupportsArray>  m_attachments;
>+  nsCOMArray<nsIMsgAttachment>  m_attachments;
Nit: don't need two spaces here.

>+    attachmentCount++;
We don't actually care.

	Joey Minta Assignee
	Comment 2 • 16 years ago

Updated to review comments.

	neil@parkwaycc.co.uk
	Comment 3 • 16 years ago

Comment on attachment 337341 [details] [diff] [review]
patch v2

>+      if(attachments.hasMoreElements())
Nit.

	Joey Minta Assignee
	Comment 4 • 16 years ago

Nit picked, and "patch v2" push as 362:8240c87b0dab