DevDisasters

Adventures in Nepotism

What's proper etiquette for handling code snafus when working for -- literally -- a mom-and-pop company?

• By Mark Bowytz
• 05/29/2014

Facing the fact that the money from his graduate research grant -- and only source of income -- was dangerously close to running out, Rumi found himself in a difficult position: How the heck would he ever be able to finish his degree and not starve in the process? There were many possibilities. He could sell blood plasma, post an ad offering a redundant vital organ on Craigslist, or perhaps get an old RV and use it to produce and sell illicit substances.

After great consideration, Rumi went with the riskiest endeavor of all: jumping headfirst into a greenfield ASP.NET project at the main headquarters of Baker's Family Restaurants Inc. as a part-time Web developer. As a bonus, he was to be paired with another developer who was quite qualified in his own right – more than 10 years of C# experience and extensive knowledge of the business processes that they'd be developing code to support.

Just kidding! He was actually a junior developer named Fabian -- a recent college grad with a coincidental genetic connection to one of the restaurant chain's corporate managers.

On the bright side, Fabian did have his own copy of "A Newbie's Guide to Developing Applications in C#," so he wasn't completely unprepared.

Mercifully, the deadline for the first project was generous and Rumi, keeping positive (and perhaps a little bit desperate), was determined to make the best of the situation.

Starting from Scratch
Initially, Rumi mentored Fabian in the basics of using Visual Studio. He explained the importance of proper source control, good patterns and practices to follow, and later escalating to how to write LINQ queries and beyond. Under expert tutelage, Fabian's skills grew, but that didn't change the fact that the deadline was still closing in.

As the crunch time loomed, the two worked increasingly independent from each other, postponing development lessons to save on time. As such, instead of regularly reviewing Fabian's check-ins, Rumi went about his work -- using the code and trusting it to work, relying on unit tests to catch any unwanted behavior, and so on. This system actually worked very well.

When Fabian decorated the Edit method in Rumi's controller with the attribute

[HandleError(ExceptionType = typeof(EmptyObjectReceivedException)] 

he was pleased to see Fabian spent time thinking about proper error handling instead of just writing code in the "nothing wrong can ever happen" style common in so many junior devs. But something about the name of the exception felt wrong.

Upon asking Fabian why he wrote a custom exception class instead of using something obvious, such as System.ArgumentNullException, he answered, "I guess we could use the existing class, but then the mail would not be sent, so I think we should keep my class."

Mail?! OK, something is definitely strange here, Rumi thought. He thanked Fabian for his explanation, but knew more digging was in order.

A Bad Ingredient
Rumi opened up Team Foundation Server and quickly found EmptyObjectReceivedException.cs -- thank goodness for good source control practices! Rumi opened the file and was greeted with this code (minus the namespace and import for anonymization purposes):

public class EmptyObjectReceivedException : Exception
{
  public string BusinessObjectType { get; private set; }
  public string BusinessObjectMethod { get; private set; }

  /// <summary>
  /// This constructor creates an exception with a generic message
  /// </summary>
  /// <param name="businessObjectType">A user-understandable name for the type of business object which could not be retrieved</param>
  /// <param name="businessObjectId">The method which received the empty parameter</param>
  public EmptyObjectReceivedException(string businessObjectType, string businessObjectMethod)
    : base(message: String.Format("An error occured when trying to retrieve a {0} in the method {1}", businessObjectType, businessObjectMethod))
  {
  this.BusinessObjectType = businessObjectType;
  this.BusinessObjectMethod = businessObjectMethod;
  SendMail();
  }

  /// <summary>
  /// This constructor lets the developer use a specific error message
  /// </summary>
  /// <param name="businessObjectType">A user-understandable name for the type of business object which could not be retrieved</param>
  /// <param name="businessObjectId">The ID of the record which could not be retrieved</param>
  /// <param name="specificMessage"> Used to initialize the Message property of the base Exception class </param>
  public EmptyObjectReceivedException(string businessObjectType, string businessObjectMethod, string specificMessage)
    : base(message: specificMessage)
  {
  this.BusinessObjectType = businessObjectType;
  this.BusinessObjectMethod = businessObjectMethod;
  SendMail();
  }

  /// <summary>
  /// Send a eMail with the exception to all administrators
  /// </summary>
  public void SendMail()
  {
  using (MailMessage EMail = new MailMessage())
  {
    OurUserRoleManagement OurUserRoleManagement = new OurUserRoleManagement(new DataContext(ConfigurationManager.ConnectionStrings["OurDB"].ConnectionString));
    foreach (OurUser admin in OurUserRoleManagement.GetAllAdmins())
    {
      EMail.To.Add(admin.EMailAdress);
    }
    
	EMail.Subject = string.Format("[" + WebConfigurationManager.AppSettings["Programname"] + "] Exception ");
    EMail.Body = "The program " + WebConfigurationManager.AppSettings["Programname"] + " throws an EmptyObjectReceivedException.\r\n The exception occured in the method " + BusinessObjectMethod + " with the object type " + BusinessObjectType + " .\r\n Here the exact exception text: " + GetBaseException().Message + " \r\n and here the stack trace: " + GetBaseException().StackTrace + "";
    EMail.From = new MailAddress(ConfigurationManager.AppSettings["OurMailAddress"].ToString(), "OurApplication");
    EMail.ReplyToList.Add(new MailAddress(ConfigurationManager.AppSettings["OurDeveloperMailAddress"].ToString()));
    EMail.SubjectEncoding = EMail.BodyEncoding = Encoding.UTF8;
    EMail.Priority = MailPriority.Normal;
    EMail.IsBodyHtml = false;
    using (SmtpClient mailhost = new SmtpClient())
    {
      mailhost.Host = "mailhost";
      mailhost.Send(EMail);
    }
  }
}

After reading the final line, Rumi calmly minimized the window and pulled up his calendar -- it was time for another "how-to" session with Fabian.

Check, Please!
"Hey Fabian, you've been doing some great code as of late -- but I checked into that method I mentioned to you earlier and, well, I still have a few questions."

Rumi shuffled through the highlighted printout of the source file, briefly considering where to begin. Should he bring up the way the exception handles itself? How about the way the code throws an exception, opens a database connection (an SMTP connection), and reads the web.config file? There's also the fact that the method, which threw the exception, is saved as a string in the exception object; code that throws the exception has to supply this information.

But these were all very technical, and most would likely call for a separate training session to fully explain why each point was so wrong -- time that they didn't have. In the end, Rumi picked what he felt to be his biggest, and most obvious, "beef" with the code.

"So, every time an exception is thrown ... an e-mail is sent out and there's no way to turn this behavior off. Are you sure this is the best approach?"

Rumi felt a little guilty and worried about hurting Fabian's feelings with his honest assessment, but oddly enough, Fabian beamed.

"Yeah! Just like you told me in our second training session: reduce duplicated code wherever and whenever possible. All exceptions flow through that code right there. Pretty cool, huh?"

Rumi frowned and puffed air out his nostrils. "All right, I'll give you credit there, but who will be receiving these e-mails? You can't assume the other admins and devs in the company want to be flooded with e-mail every time an exception is hit."

"Really, it's no big deal, it's just going to one person, the project sponsor."

"Wha? Come again? Have you cleared that yet? I mean, that'll potentially result in a lot of e-mails …"

"Oh, I know, but like I said, it's cool," Fabian interjected, "I set up a message filter in her Outlook and, besides, if something goes really bad, it's not like she'll come down hard, I mean she's my mom, after all."

OOHH! Now the pieces had fallen into place. Rumi had met the project owner -- a department head who had never written a line of code in her life -- on only a few occasions, but never put two and two together. Now, more than ever, called for professional tact.

"Well, let's not give her that chance to get upset. Let's take a minute to talk about the benefits of writing to a log file…"