Group Code By Responsibility

Please take a look at the following method.

  private void SendCustomerWelcomeEmail(Customer customer)
  {
     var smtpHost = ConfigurationManager.AppSettings["SmtpHost"];
     StringBuilder builder = new StringBuilder();
     builder.AppendLine($"Hi {customer.FirstName},");
     builder.AppendLine();
     builder.AppendLine("Thank you for signing up to our newsletter.");
     builder.AppendLine();
     builder.AppendLine("Yours sincerely,");
     builder.AppendLine("The ABC Finance Customer Services Team");
     string welcomeEmailBody = builder.ToString();
     var smtpClient = new SmtpClient(smtpHost);
     string welcomeEmailSubject = "Welcome to ABC Finance!";
     MailMessage email = new MailMessage("info@abcfinance.example.com", 
                                         customer.EmailAddress,
                                         welcomeEmailSubject,
                                         welcomeEmailBody);
     smtpClient.Send(email);
  }

What does SendCustomerWelcomeEmail() do? Sure, it sends a customer welcome email. On closer inspection, though, it does more than the sending. It also formats the email. 

Since a function should do only one thing, SendCustomerWelcomeEmail() already does too much.

Not only does it do two things—formatting the email message and then sending it—but the code generating these two responsibilities is also intermingled!

Here is the method again, but with the problematic lines highlighted:

  private void SendCustomerWelcomeEmail(Customer customer)
  {
     var smtpHost = ConfigurationManager.AppSettings["SmtpHost"];
     StringBuilder builder = new StringBuilder();
     builder.AppendLine($"Hi {customer.FirstName},");
     builder.AppendLine();
     builder.AppendLine("Thank you for signing up to our newsletter.");
     builder.AppendLine();
     builder.AppendLine("Yours sincerely,");
     builder.AppendLine("The ABC Finance Customer Services Team");
     string welcomeEmailBody = builder.ToString();
     var smtpClient = new SmtpClient(smtpHost);
     string welcomeEmailSubject = "Welcome to ABC Finance!";
     MailMessage email = new MailMessage("info@abcfinance.example.com", 
                                         customer.EmailAddress,
                                         welcomeEmailSubject,
                                         welcomeEmailBody);
     smtpClient.Send(email);
  }

1. The first line retrieves the SMTP host information from the configuration. So why is variable smtpHost declared and assigned to yet not used for nine lines? As it stands, the line assigning to smtpHost is mixed in with the email formatting code. How confusing—Yuch! We would be better off putting this first line closer to when we need smtpHost—i.e. when we’re sending the email. It’s not doing much good at its current location.
2. Similarly, smtpClient is also declared and assigned among email message formatting code. Like with smtpHost, we should set up smtpClient closer to when we need it to send the email.

  private void SendCustomerWelcomeEmail(Customer customer)
  {
     // Format email
     StringBuilder builder = new StringBuilder();
     builder.AppendLine($"Hi {customer.FirstName},");
     builder.AppendLine();
     builder.AppendLine("Thank you for signing up to our newsletter.");
     builder.AppendLine();
     builder.AppendLine("Yours sincerely,");
     builder.AppendLine("The ABC Finance Customer Services Team");
     string welcomeEmailBody = builder.ToString();
     string welcomeEmailSubject = "Welcome to ABC Finance!";
     MailMessage email = new MailMessage("info@abcfinance.example.com", 
                                         customer.EmailAddress,
                                         welcomeEmailSubject,
                                         welcomeEmailBody);
   
     // Send email
     var smtpHost = ConfigurationManager.AppSettings["SmtpHost"];
     var smtpClient = new SmtpClient(smtpHost);
     smtpClient.Send(email);
  }

The email formatting and the email sending now reside in separate, contiguous code blocks.

Please note that this kind of code grouping is usually a precursor for extraction into smaller helper methods.

Let’s try that here:

  private void SendCustomerWelcomeEmail(Customer customer)
  {
     var email = FormatCustomerWelcomeEmail(customer);
     SendEmail(email);
  }

Nice! We also managed to get rid of the code comments. Why?

Because now the helper function names clearly explain what they do. And this is always better than using comments to achieve the same objective. 

Ideally, functions should do only one thing. However, in brownfield codebases frequently, they don’t.

In those cases, we can help by grouping code by responsibility before extracting it into helper functions.