HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Two overloaded methods to move an e-mail message to another mailbox folder

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
overloadedmessagemailboxmovemethodstwomailanotherfolder

Problem

I have an API for managing an email mailbox. There is a class called Message, and it has two Move overloads:

/// 
/// Move this message to the mailbox folder whose path is provided.
/// 
/// The path to the mailbox folder to which this message will be moved.
/// If true, the destination folder will be created first if it doesn't exist.
/// 
/// An exception will be thrown if this message does not reside in a mailbox because
/// only messages in mailboxes can be moved between folders.  Use  to determine if
/// the message resides within a mailbox and folder; that value will be null if it does not.
/// An exception will be thrown if the destination folder does not exist and if the provided  is false.
/// 
public void Move(string toFolderPath, bool createFolder)
{
    ValidateMoveCopy();
    Move(GetCreateFolder(toFolderPath, createFolder));
}

/// 
/// Move this message to the provided mailbox folder.
/// 
/// The mailbox folder to which this message will be moved.
/// An exception will be thrown if this message does not reside in a mailbox because
/// only messages in mailboxes can be moved between folders.  Use  to determine if
/// the message resides within a mailbox and folder; that value will be null if it does not.
public void Move(MailboxFolder toFolder)
{
    ValidateMoveCopy();
    ParentMailbox.MoveMessage(this, toFolder);
}


The first overload will retrieve the mailbox folder or create it if it doesn't yet exist, but only if the createFolder argument is true; then it will just call the second overload.

What I'd like to avoid is calling the ValidateMoveCopy twice when the first overload is used (once in that method, and once in the second overload); however, I don't want GetCreateFolder to be called in the first method if validation fails.

Here are GetCreateFolder and ValidateMoveCopy:

```
///
/// Return the folder with the provided path, if it exists, from the parent mailbox,
/// or else create it if it should be created based

Solution

I think the best option would be to introduce a third, private method:

private void Move(MailboxFolder toFolder, bool requiresValidation)
{
    if (requiresValidation)
    {
        ValidateMoveCopy();
    }
    ParentMailbox.MoveMessage(this, toFolder);
}


Then your other methods just call that one.

public void Move(MailboxFolder toFolder)
{
    Move(toFolder, true);
}

public void Move(string toFolderPath, bool createFolder)
{
    ValidateMoveCopy();
    MailboxFolder toFolder = GetOrCreateFolder(toFolderPath, createFolder);
    Move(toFolder, false);
}


I renamed GetCreateFolder to GetOrCreateFolder which is slightly easier to read IMO. I've also introduced a variable for toFolder as I don't like lines of code that do 2 things.

Edit:

Short and concise methods are brilliant and a very good thing to aim for. However, consider your method at a glance:

Move(GetCreateFolder(toFolderPath, createFolder));


How easy is it to spot the side effect there (possibly creating a new folder) vs my version:

MailboxFolder toFolder = GetOrCreateFolder(toFolderPath, createFolder);
Move(toFolder, false);


It's a stylistic thing without doubt and if you prefer the brevity then that's absolutely fine. I should have been more explicit about that being a preference thing rather than a rule!

Code Snippets

private void Move(MailboxFolder toFolder, bool requiresValidation)
{
    if (requiresValidation)
    {
        ValidateMoveCopy();
    }
    ParentMailbox.MoveMessage(this, toFolder);
}
public void Move(MailboxFolder toFolder)
{
    Move(toFolder, true);
}

public void Move(string toFolderPath, bool createFolder)
{
    ValidateMoveCopy();
    MailboxFolder toFolder = GetOrCreateFolder(toFolderPath, createFolder);
    Move(toFolder, false);
}
Move(GetCreateFolder(toFolderPath, createFolder));
MailboxFolder toFolder = GetOrCreateFolder(toFolderPath, createFolder);
Move(toFolder, false);

Context

StackExchange Code Review Q#111697, answer score: 5

Revisions (0)

No revisions yet.