Staying DRY

A Real Example Of Refactoring Duplicate Code That I Didn't Know I Had

by JamesQMurphy | September 2, 2019

At almost every company I have worked at, when a new software developer joined the team, they were immediately given an "easy" task --- something like correcting a spelling mistake, or changing a font size. This is a common practice since it helps a new team member get their development environment up and running, and it introduces them to the specifics of the team's development process.

So imagine it's your first day, and you are asked to change the display style for the titles of blog posts, from <h1> to <h1 class="display-4">. Easy enough, right? If the code looked anything like the code for this blog, you would find the file Views/Blog/Details.cshtml and make the change, just like I did as part of Release v0.2.3:

@model JamesQMurphy.Blog.Article
@inject JamesQMurphy.Blog.IMarkdownHtmlRenderer Renderer

<h1 class="display-4">@Model.Title</h1>
@if (!string.IsNullOrWhiteSpace(Model.Description))
{
    <h5>@Model.Description</h5>
}
<h6 class="text-muted font-weight-light"><small>by JamesQMurphy | @Model.PublishDate.ToString("MMMM d, yyyy")</small></h6>
<br/>
<div class="jqm-easy-to-read">
    @Html.Raw(Renderer.RenderHtml(Model.Content))
</div>

And it would work just fine:

view of blog with new Title styling in place

Time for lunch, you think, as you mark the task as "Completed" and assign it to the QA Team. What a great new job! And as you eagerly await your next task, you feel pretty good. Until, of course, the QA Team fails your change because it doesn't show up on the Privacy page:

view of Privacy page with the old Title styling

"The Privacy Page?" you exclaim. Sure enough, you see it once you look in the file Views/Home/Privacy.cshtml:

@{
    ViewData["Title"] = "Privacy Policy";
    var webSiteTitle = Configuration.GetValue(typeof(string), "WebSiteTitle");
}
<article>
    <h1>@ViewData["Title"]</h1>

    <p>
        @webSiteTitle ("The Website") does not collect any personal information, nor does it employ any use of "tracking" cookies.
        The Website serves as my personal blog, and a demonstration of how to deploy a web site using DevOps best practices.  At
        this time, the Website is in a "read-only" mode, so there is no way to send <i>any</i> data to the site, let alone personal
        data.  If this changes in the future, then this policy will be updated, and a mechanism will be employed to let users
        know of the updated policy.
    </p>
    <p>
        The Website is hosted on Amazon Web Services (AWS) using their <a href="https://aws.amazon.com/api-gateway/">API Gateway</a>
        service.
    </p>
</article>

And now that you know better, you don't stop there. You do a quick search of the source code, and sure enough, you find another instance of the problem --- this time, in the About page. Welcome to the world of WET Code1; i.e., code that violates the DRY Principle.

Officially, the DRY ("Don't Repeat Yourself") principle states that "Every piece of knowledge must have a single, unambiguous, authoritative representation within a system." And although this example is somewhat contrived, it is a violation. The Privacy and About pages use the same styling as the blog pages do, but the layouts are repeated in the Views/Home/Privacy.cshtml and Views/Home/About.cshtml views. Changing the style of one will most likely require changes to the other two.

So what do you do? If it's your first day, or even your first week, chances are you won't have enough of an understanding of the code to create a proper abstraction (although you can ask a senior developer). One solution would be to define a new style (like articleTitle or something) and use that instead of display-4. That would eliminate one aspect of the repetition --- additional changes to the articleStyle style would only require a change in one place. Or maybe you would do something else? Maybe your manager tells you to just go ahead and make the changes in the other places; perhaps there is a redesign in the works?

In practice, it's not always easy to identify how or even when code has violated the DRY Principle. In my case, I had not even realized that the "About" and "Privacy" pages even had a problem. I knew that the styles were inconsistent, but I had that on a list of cosmetic changes that I was planning to do. Instead, it was something completely different that made me truly aware of the problem --- my desire to include Mermaid Flowcharts.

Displaying Flowcharts With Mermaid

Mermaid is a third-party package that lets you create flowcharts with client-side Javascript, like this:

graph LR A-->B B-->C

Using Mermaid is simple; all you need to do is define a <div> block that contains Mermaid's rendering language, and then add a Javascript call. For example, the following HTML block renders the diagram above:

<div class="mermaid">
  graph LR
    A-->B
    B-->C
</div>

And since I chose Markdig for rendering blog posts, support for generating Mermaid blocks is included. If a Markdown code fence is labeled as "mermaid," Markdig will render the <div> block instead of the normal <pre> and <code> tags. To produce the HTML block above, all I need to type in the Markdown source is this:

```mermaid

graph LR
  A-->B
  B-->C

```

When I first learned about Mermaid, I knew right away that I wanted to include this functionality in my blog posts. But I also wanted it on the About page, where Mermaid could come in handy to illustrate the CI/CD process of this website. And that would mean making the same change to two different views.

In short, that's when I realized I needed to refactor the About (and Privacy) pages to use the same rendering mechanism as the blog pages.

How I DRYed The Code

When I originally planned to include Mermaid functionality, I was going to include the refactoring process in the same release. But I wanted to highlight the violation of the DRY principal, and how I fixed it. So I split the effort into two releases:

Refactoring the code involved creating a new view named Article.cshtml. You'll also notice that I took this opportunity to replace ViewData key names with constants; this was another form of violating the DRY principal (repeating key names) that I wanted to eliminate.

@model JamesQMurphy.Blog.ArticleMetadata
@inject JamesQMurphy.Blog.IMarkdownHtmlRenderer Renderer

@{
    var title = String.Empty;
    if (ViewData.ContainsKey(Constants.VIEWDATA_PAGETITLE))
    {
        title = ViewData[Constants.VIEWDATA_PAGETITLE].ToString();
    }
    if (Model != null)
    {
        if (!String.IsNullOrWhiteSpace(Model.Title))
        {
            title = Model.Title;
        }
    }
}

<article>
    @if (!String.IsNullOrWhiteSpace(title))
    {
        <h1 class="display-4">@title</h1>
    }
    @if (Model != null)
    {
        @if (!String.IsNullOrWhiteSpace(Model.Description))
        {
            <h5>@Model.Description</h5>
        }
        <h6 class="text-muted font-weight-light"><small>by JamesQMurphy | @Model.PublishDate.ToString("MMMM d, yyyy")</small></h6>
    }
    <br />
    @if (ViewData.ContainsKey(Constants.VIEWDATA_MARKDOWN))
    {
        @Html.Raw(Renderer.RenderHtml(ViewData[Constants.VIEWDATA_MARKDOWN].ToString()))
    }
</article>

I'll admit that it's not perfect; there's a few too many if statements for my liking. But the view works as advertised for both blog posts and the About and Privacy pages. Plus, this is the one and only instance of the <h1 class="display-4"> tag --- you can't get any more DRY than that!

view of Privacy page with the new Title styling

But more importantly, this refactoring effort allowed me to add Mermaid much more easily, since I only had one view to modify.

Conclusion

If it wasn't already evident, I feel that the DRY Principle is one of the more important principles in software development. But note that you do not have to be fanatical; you just need to be disciplined enough to eliminate it when you see it. Code development, like any major design effort, is an iterative process. Like me, you may not even see the duplication until it's time to add a new feature. The "Privacy" page has been in the project since its inception, but I intentionally gave no thought to its styling or even its content until I was ready. There was no way I would have thought to combine it with the About page and the blog pages.

One thing I will point out: following the DRY principle is much, much easier when you have unit tests in place. Staying DRY is often a refactoring exercise, and having that bank of successful unit tests allows you to refactor with confidence. I'll have more to say about unit tests in upcoming posts.


  1. "WET" can stand for something clever like "Write Everything Twice" or "Write Every Time."

 

Comments


 

Want to leave a comment? Sign up!
Already signed up? Sign in!