CodeSOD: Shorely a Bad Choice

"This was developed by the offshore team," is usually spoken as a warning. There are a lot of reasons why the code-quality from offshore teams has such a bad reputation. You can list off a bunch of reasons why this is true, but it all boils down to variations on the Princpal-Agent Problem: the people writing the code (the agents) don't have their goals aligned with your company (the principal).

Magnus M recently inherited some C# code which came from the offshore team, and it got principal-agented all over.

/// <summary>/// License Person CompanyName/// </summary>private string ErrorMsg{ get{return "It is not possible to connect to the license server at this time." + Environment.NewLine+ Environment.NewLine + "Please try again later or contact customer service for help at info@domain.com" + Process.Start("mailto:info@domain.com");}}

When I started reading this code, I just got annoyed at the Environment.NewLine calls, and was thinking about how formatting an error message like this right in the code is such an awful code smell, but it's hardly a WTF- until I got to Process.Start("mailto:info@domain.com").

As the name implies, Process.Start starts a process. It's normally used to execute external programs, but here we pass a URL to it. Since this software runs on Windows, it should trigger the OS to open the default mail program, if there's one assigned. If there isn't, your attempt to access the ErrorMsg property just threw an unhandled exception.

There is no sensible reason why accessing a read-only property should launch a mail program. Even if "hey, just start a mail program when things go wrong," were an acceptable UX choice (spoilers: it isn't), this is so far away from "single responsibility principle" that it makes my head hurt.

Magnus adds:

There are many, many issues with the code, but I thought this snippet was a good representation of the general quality. ... My favorite detail is probably the comment.

I don't know about you, but "License Person CompanyName" is actually the name I put on all my Git commits.

[Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!