Bug Recipe: Double Duty View and a Misguided Property Name

At work I've picked up an existing project and have been adding features to it. This has been a whole new thing for me since most of the stuff I've worked on in the past has been projects of my own creation. I recently got caught with my pants down when I let a bug slip over to the QA people. I got bitten because I trusted that a property name did what it said it did and because I didn't thoroughly test the possible scenarios around this particular feature. It's my fault for letting this by, but I just wanted to highlight how important naming really is.

It all started with a simple user story: User would like to track the expiration date of bacon.

Simple enough, I opened up the view model and added my ExpirationDate property.

    public class BaconViewModel {

        public virtual string Name { get; set; }

        public virtual DateTime? ExpirationDate { get; set; }

        // A lot more code here....

        public bool IsNew{
            get { return String.IsNullOrEmpty(Name); }

Up to this point, creating bacon really only had one property to fill out, the Name. What I failed to notice was that little devil of code in the IsNew property. Does a view model not having anything in the name really make it new? I'm getting a bit ahead of myself though, so ponder on that last question while I continue to tell my story.

The IsNew property itself isn't the only ingredient to this bug. The other part stems from the fact that in the project I inherited, the view does double duty for create and edit. Here's the basic view code before I added my feature:

        <% if(Model.IsNew) %>  
            Creating Bacon is the most noble of all tasks.
        <% else %>
            Be careful editing bacon.

    <% using (Html.BeginForm()) {%>
        <%: Html.ValidationSummary(true) %>

            <div class="editor-label">
                <%: Html.LabelFor(model => model.Name) %>
            <div class="editor-field">
                <%: Html.TextBoxFor(model => model.Name) %>
                <%: Html.ValidationMessageFor(model => model.Name) %>
                <input type="submit" value="Sizzle" />

    <% } %>

Of course this is all just a contrived example, but do you see that bit of code in the h2 tag? If we're creating bacon we want to show one thing and if we are editing then we want to show another. I went about my business and added the UI code to capture that bacon expiration date. When I did all of this I introduced a nice little bug.

What happens when I'm creating bacon and give it a name, but fail to give it a good expiration date? The post happens like normal and a partial view model gets filled out and the ModelState has errors. We throw that partially completed view model back to the view along with the errors so that the user can weep at their awfulness. Except this time that IsNew property isn't feeling so new anymore and now the view goes into edit mode and thoroughly confuses the user. Witness the carnage:

Like I said above, this is a contrived example. What really happened in my case was an exception when I tried to grab the value of the nullable property. Hopefully I've made my point though. The two lessons I'm taking from this experience is:

  1. Don't have views that do double duty. If you have parts that are the same, you can use partials to keep things DRY.
  2. Make sure that your property names do what they say they do all of the time and not only just out of coincidence.

No small animals got killed because of this bug obviously, but I did take a nice punch to the ego when QA found a fat bug due to an enhancement I made. That will just be one more thing I know to check when I'm spelunking in other people's code. Happy coding!

  • Jessie Lewis

    I dont have even one single clue what you just said. I even tried to pretend like it didnt matter that I didnt understand what you were talking about and just tried to follow your story but that too was impossible. I am assuming you program but made an error but the details to me are like trying to read chinese. Im not trying to insult you. Obviously you are very skilled at something that I know nothing about lol. Good luck with your bacon!!!!

  • http://calvinbottoms.blogspot.com Calvin Bottoms

    Love the part about bits of code that match their names only by coincidence. Nicely told. Welcome to the world of legacy!