Question

I’m trying to refactor some “sending out an email” code by dividing the steps (validate, attach related content, format, send) into separate classes that can be more easily tested, logged, and updated.

As part of this I’ve got to figure out a way for the operations to communicate validation or transient errors (“that item was deleted”) back to the originator so that it can ask the user for more info or tell them the bad news. Here is the path the thread takes (yay, a doodle)

     "Controller"                     
      .   -> Outbox 
      .         -> Validator 
      .         -> Formatter 
      .         -> Sender
      .   <- 

                   -> Parameters, work in progress
                   <- Good, not so good, "you better sit down" news

So you're a thoughtful sort, between “return”, “exceptions”, or “context”… which one makes you happiest?

A. Throw an exception at any problem and let the controller divide up the ones it can handle gracefully and the ones that know my “beeper”.

B. Return some sort of Result<T>class to carry both the product of the operations (an email) and the enumerated results of various operations.

C. Pass a context into/outof all the steps where they can indicate any parameters they can’t deal with and keep the method signatures very straightforward.

D. Son, you’re overthinking this.. Here is what you’re gonna do : <YourSpecialJujuHere/>

Thanks for any and all contributions, you rock in concert.

Was it helpful?

Solution

You could use the Template Method pattern with the Strategy pattern:

Your Controller becomes a Template Method. For each step of the email sending process, you call out to a delegate/Strategy class that implements the step.

public class EmailSender
{
    private iOutboxGetter outboxGetter;
    private iMsgValidator validator;
    private iMsgFormatter formatter;
    private iMsgSender    sender;

    //setters for each stragegy, or a constructor
    //good use for IOC container

    public iSendResult SendMessage(iMsgParams params)
    {
        try
        {
            var outbox = outboxGetter.getOutbox(params.outbox);
            var validationResults = validator.validate(params);
            if(validationResults.IsValid)
            {
                var msg = formatter.formatMsg(params.message);
                sender.send(msg);
                return new AllGoodSendResult();
            }
            else
            {
                return new ValidationFailedSendResult(validationResults);
            }
        } 
        catch(CatastrophicException e)
        {
           Pager.SendCriticalPage(e.message);
            return new CatistrophicFailureSendResult(e);
        }
    }
}

I prefer to use exceptions when the code must deviate from the Happy Path. I feel they keep the logic and the error handling cleanly separated.

Edit: The return from the SendMessage method indicates to the caller whether validation passed or not, and what failed in validation. The caller can then prompt the user for more information and retry, or indicate success. An exception is only thrown in the event of a truely exceptional circumstance.

Using this approach, each component of your algorithm can be mocked and tested independently and no Strategy needs to know how any other Strategy works, nor does it need to know how to handle someone else's errors. Finally, all of your error handleing is centeralized in a single place.

OTHER TIPS

Maybe the problem lies in the sequencing of actions, that makes an action call the next.

A different approach is to make the Controller call all actions in turn. In that case, there is a direct relation between your Controller and each action.

Each action may return a simple result, or signal an error via a way appropriate to its case:

  • Exceptional situations via an exception
  • No result via a null return.
  • ...

Reuse from one action to another can happen as local variables.

Sample code (add parameters and so on as needed) :

    class Controller1 {

       private Sender sender = new SenderImpl();

       public void process(String text) {
         try {
           Outbox box = getOutbox();
           List<Errors> errors = validate(text);
           if (!errors.isEmpty()) {
             ....
             return;
           }
           String formatted = format(text);
           sender.send(formatted);
         } catch(MyException e) {
           ....
         }
       }
    }

Although in this code, the steps are delegated to method of the same class, it is very easy to follow the same structure with instances of other classes (but only as needed, don't over-engineer). As you mentionned, this can be justified for testability. I changed the sample code for sender.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top