Tag Archives: Design

Testing Model Validations?

2 Replies

 

ASP.NET MVC 2 allows you to do data validation, either using the de-facto Data Annotations or plugging in your own, much in the same way xVal work. If you’re interested in seeing how that works, take a look at this post.

This post however concerns testing. Here’s some code:

[AcceptVerbs(HttpVerbs.Post)]
public ActionResult Create(Customer customer)
{
    if (ModelState.IsValid)
    {
    return RedirectToAction("Index");
    }
    return View(customer);
}

.csharpcode, .csharpcode pre
{
font-size: small;
color: black;
font-family: consolas, “Courier New”, courier, monospace;
background-color: #ffffff;
/*white-space: pre;*/
}
.csharpcode pre { margin: 0em; }
.csharpcode .rem { color: #008000; }
.csharpcode .kwrd { color: #0000ff; }
.csharpcode .str { color: #006080; }
.csharpcode .op { color: #0000c0; }
.csharpcode .preproc { color: #cc6633; }
.csharpcode .asp { background-color: #ffff00; }
.csharpcode .html { color: #800000; }
.csharpcode .attr { color: #ff0000; }
.csharpcode .alt
{
background-color: #f4f4f4;
width: 100%;
margin: 0em;
}
.csharpcode .lnum { color: #606060; }

This code checks to see if my model is valid. If it is, it then saves it (not in the code) and redirects to the index action, thus producing a RedirectToActionResult. If it is not valid, it will return a ViewResult. Here’s the model:

public class Customer
{
    [Required]
    public string FirstName { get; set; }
    [Required]
    public string LastName { get; set; }
}

.csharpcode, .csharpcode pre
{
font-size: small;
color: black;
font-family: consolas, “Courier New”, courier, monospace;
background-color: #ffffff;
/*white-space: pre;*/
}
.csharpcode pre { margin: 0em; }
.csharpcode .rem { color: #008000; }
.csharpcode .kwrd { color: #0000ff; }
.csharpcode .str { color: #006080; }
.csharpcode .op { color: #0000c0; }
.csharpcode .preproc { color: #cc6633; }
.csharpcode .asp { background-color: #ffff00; }
.csharpcode .html { color: #800000; }
.csharpcode .attr { color: #ff0000; }
.csharpcode .alt
{
background-color: #f4f4f4;
width: 100%;
margin: 0em;
}
.csharpcode .lnum { color: #606060; }

Given the previous, what would you expect the following test to do? Pass or fail?

    [Fact]
    public void wont_give_you_any_hints()
    {
    var controller = new CustomerController();
 
    var customer = new Customer();
 
    customer.FirstName = "Jak a.k.a. Casey";
    customer.LastName = String.Empty;
 
    var result = controller.Create(customer);
 
    Assert.IsType(typeof(ViewResult), result);
    }

.csharpcode, .csharpcode pre
{
font-size: small;
color: black;
font-family: consolas, “Courier New”, courier, monospace;
background-color: #ffffff;
/*white-space: pre;*/
}
.csharpcode pre { margin: 0em; }
.csharpcode .rem { color: #008000; }
.csharpcode .kwrd { color: #0000ff; }
.csharpcode .str { color: #006080; }
.csharpcode .op { color: #0000c0; }
.csharpcode .preproc { color: #cc6633; }
.csharpcode .asp { background-color: #ffff00; }
.csharpcode .html { color: #800000; }
.csharpcode .attr { color: #ff0000; }
.csharpcode .alt
{
background-color: #f4f4f4;
width: 100%;
margin: 0em;
}
.csharpcode .lnum { color: #606060; }

I’ll give you a hint. It fails. Now I understand why it fails. My problem however is that for me to test my model is valid is going to force me to change the way I have to write my code, and potentially make it less readable.

Problem with Client-Side Validation in MVC 2

3 Replies

 

[Note: This post applies to ASP.NET MVC 2, Preview 2.0]

Today while doing the demo for the previous post, I ran into an issue where the Javascript code for the client-side validation (the call to EnableClientValidation ) was not being output during the form rendering.

Take a look at the following two snippets:

First: 

    <% Html.BeginForm();%>

 

        <%=Html.EditorForModel() %>

            <p>

                <input type="submit" value="Save" />

            </p>

    <% Html.EndForm(); %>

 

Second: 

  <% using (Html.BeginForm()) {%>

 

        <%=Html.EditorForModel() %>

            <p>

                <input type="submit" value="Save" />

            </p>

    <% } %>

 

See the difference? The latter is using the using statement. In my previous post I’m using this option and all works well. However, if you go with the first option, the Javascript call will not be output. The reason for this (after debugging the source) is that the call to make this happen takes place in the Dispose method of the MvcForm. Explicitly calling Html.EndForm won’t cause this to take place.

I’ve talked to Mathew from the QA team, and he’s confirmed it’s a known issue. I think the output of the JS code should ideally be decoupled from the form construction. For instance, If I were to use a manual form tag, this wouldn’t work either [I’ve haven’t given it that much thought either].

In the meantime, if you want client-side validation, make sure you use the second option.

Client-Side Validation in MVC 2

4 Replies

 

ASP.NET MVC 2 Preview 2 now ships with client-side validation built into the box. It follows the same idea as the xVal framework whereby you can define validation rules once and have them enforced both on the server and the client.

By default, MVC uses Data Annotations which is available in System.ComponentModel.DataAnnotations on the server and the jQuery Validator plugin on the client. Much like xVal you can customize these to use whatever you want. I’m preparing some demos for next week of how to do client-side validation and since there isn’t much info on it, I’ve decided to post it.

Server-Side Validation

Server-side validation with Data Annotations works without having to take any additional steps. You decorate your model using attributes and the model binder uses this information to set the ModelState. 

    public class Customer

    {

        [Required(ErrorMessage = "First name is required")]

        public string FirstName { get; set; }

        [Required(ErrorMessage = "Last name is required")]

        public string LastName { get; set; }

        public string Email { get; set; }

    }

 

On the server-side you would do the usual to check if the model state is valid and if not display validation messages back to the client: 

        [AcceptVerbs(HttpVerbs.Post)]

        public ActionResult Edit(int id, Customer customer)

        {

            if (ModelState.IsValid)

            {

                // TODO: Do whatever...

                return RedirectToAction("Index");

            }

            return View(customer);

        }

 

The form is: 

    <%= Html.ValidationSummary() %>

    <% using (Html.BeginForm()) {%>

 

        <%=Html.EditorForModel() %>

            <p>

                <input type="submit" value="Save" />

            </p>

    <% } %>

 

(I’m using the EditForModel which will automatically generate an input field for each property of the model. If you want finer control you can spit out individual fields or use templates).

Client-Side Validation

Client-Side validation kicks in only if it is explicitly activated in the view. To do this, you need to call EnableClientValidation as shown below: 

 

    <% Html.EnableClientValidation(); %>

    <%= Html.ValidationSummary() %>

    <% using (Html.BeginForm()) {%>

 

        <%=Html.EditorForModel() %>

            <p>

                <input type="submit" value="Save" />

            </p>

    <% } %>

 

By doing this, when the form is generated, the MVC framework will add a call to JS function EnableClientValidation passing in the correct parameters based on the data annotation attributes defined on the model. Last but not least, we need to include 3 Javascript files in the View (I normally put them in Site.Master). 

    <script src="../../Scripts/jquery-1.3.2.min.js" type="text/javascript"></script>

    <script src="../../Scripts/jquery.validate.min.js" type="text/javascript"></script>

    <script src="../../Scripts/MicrosoftMvcJQueryValidation.js" type="text/javascript"></script>

 

Make sure you include the last file, where EnableClientValidation is defined. That’s all there is to it. Once you run this, your app will have both client side and server side validation using the default Data Annotations and jQuery Validator.

Download demo from here

If you’re going to provide RAD for ASP.NET MVC, change your thinking hat

5 Replies

ASP.NET MVC is not new. Ignoring the pattern for a moment, the way things work dates back to the days of CGI’s and ISAPI’s, where you had to role out pretty much everything by hand. You would get your data, format it and send it back to the browser, producing HTML. It wasn’t easy, but you did have one advantage. You had full control.

 

Gradually, tools starting to emerge to bring some sort of “RADness” to this kind of development. I in particular, used a technology called WebBroker from Delphi; we had Actions, PageProducers (replaced tags at runtime in an HTML template) and our data. Same concept as ASP.NET MVC, same concept as MonoRail, and other frameworks. Despite still having to do a lot of things manually, it did prevent us from having to write repetitive and tedious code in many cases.

 

Up to then, you had one requisite to develop web applications: you had to understand the web and the technology behind it.

 

Traditional ASP.NET or ASP.NET WebForms came around, among other things, to solve this problem: allow the non-web developer to leverage their existing knowledge to port their applications to the web. This meant not having to know much about web technology and at the same time. ASP.NET WebForms provided a powerful approach to web development: the concept of drag-n-drop that existed in WinForm applications. You could now take a control, drag it across to a form, set some properties and away you go: in minutes you had a web interface without having to even know what HTML meant.

 

Now, I’m no fan of ASP.NET WebForms. Actually that’s not entirely true. I hate it, but I still admit that for some it has it’s attractiveness and has solved problems for many people and put food on the table for many developers. So it has it’s place and will continue to live on.

 

I’m also not alone. There are others like me that don’t like ASP.NET WebForms. As such, when ASP.NET MVC came out, it gave us another technology, based on the ASP.NET stack, to  develop web applications.  Ignoring many of the differences and advantages it has over traditional ASP.NET, above all, it gave us control. As I always say, it put the web back into web development.

 

As developers started to embrace this technology, questions started to arise about what kind of support there would be in terms of controls. Naturally, third party component vendors, those same ones that were providing support for traditional ASP.NET components, were being asked about this too. All those that I spoke to pretty much said the same thing, let’s wait and see. And it’s completely the right approach. There’s no point in investing a large amount of time and resources into supporting a new framework if you don’t know what the success of the framework will be, both in adoption, as well as commitment from the company behind it, in this case Microsoft.

 

Now that ASP.NET MVC has been released, and there is a growing number of developers moving over from traditional ASP.NET, some companies are starting to develop a limited number of controls for ASP.NET MVC. I’m guessing that some of these companies are dipping their feet into the MVC waters to see what kind of success their components might have, before taking the big plunge.

 

I’ve examined a few of these components. I’m not going to mention any names or give an examples, but suffice to say, that up to now, I don’t like what I see. And the reason is simple. Most of them are taking the wrong approach. They are thinking with their WebForms hat on. However, since there is no visual designer, they are using an immense amount of markup with custom tags to define their components. And many are mixing data, behavior and appearance in these markups. The problem with this approach is that it’s going down the same route that many ASP.NET MVC developers left behind from by moving away from traditional WebForms: rich-intelligent-know-it-all controls. I’m not going to show any code since I don’t want to single-out any specific control/company, but sample applications for many of these exist and you’re free to examine them for yourself.

 

To summarize, trying to bring the same RAD controls that exist in WebForms over to ASP.NET MVC might not necessarily be correct thing to do. This is a different way to doing web development, a different mentality. And in my humble opinion, I think that this might backfire, since MVC developers mostly will not be too keen on this type of solution, the adoption rate for the components will be low and the component vendors behind them will throw in the towel.

 

ASP.NET MVC “Models”: The lonesome folder!

3 Replies

ASP.NET MVC comes with a Models folder but no Model. Although some see this as a shortcoming, I'm actually delighted that the framework does not bind me to a specific data technology or pattern. If I want to use Active Record I can use Castle's ActiveRecord. If I want to use a Repository Pattern, I can use NHibernate. If I want to throw something together for a quick prototype, I can use Linq 2 Sql. The advantage of not being tied to a specific data access pattern for me is a win situation.

In many a demo, it's been indicated that the  "Models" folder contains your model, whatever that happens to be. It could be your Domain Model, your L2S entities, etc.  Now there are obvious reasons that whatever model you have, you wouldn't include it into your ASP.NET MVC application directly. You would probably reference it as an external assembly, and not directly place it in the Models folder.

 

image

 

So this begs the question. Do we just remove the Models folder? Well before I answer that, let me dig into something else.

 

Binding in ASP.NET MVC

We've seen many demos and tutorials where our model is directly passed in into our actions. We would have something along the lines of:

 

image

 

ASP.NET MVC's binding is pretty powerful. It can reflect on the properties of your class (even if they contain complex types) and match these up to the properties that come through over the wire, via the request POST. As long as you name the properties the same, it works like a charm. One issue that this introduces, as outlined by Justine in his post, as well as many others, is that the binding can come back to bite you. If you Customer entity has a field that you don't want the user to be able to update, you'll need to explicitly specify this by using attributes:

 

image

 

However, as simple as it might seem, it's error prone and violates DRY. Alternatives to this approach include creating custom model binders or using interfaces to specify the properties that you want to be databound. But let's hold that thought for a moment.

 

A typical update scenario for a typical entity

The following screen has a sample form to fill in the details for a customer

image

It's pretty standard. A bunch of fields and some dropdowns. But how is the dropdown values set? You could set them in the controller action just before displaying the view, using ViewData:

 

image

 

Now I don't know about you, but for me, using ViewData is a smell. If we don't use ViewData to pass information about our model, why should we be using it to pass information that our model indirectly consumes? And of course, being a dictionary that it is, it's prone to errors.

 

One plus one makes habitants for Models folder

We have two problems to solve:

  • Binding to only those properties that we need
  • Passing additional information to our views in a strongly-typed manner

Well this just cries out for a new class to encapsulate this information, i.e. a view model. Instead of binding directly to entities, you can bind to a new class that exposes only the information that is allowed to be updated. You can also use this class to provide other information required for a particular view. In the case of Customer, we could end up with something along the lines of:

 

image

 

In this case, we are using CustomerData as a DTO to hold the customer data (Name, Email, etc.) and the CustomerCreateViewModel as the View Model to hold other pertinent information for that particular view, for instance a list of countries. The action would now pass in this model as opposed to Customer:

 

image

 

We now have a strongly-typed model to represent all the information we need for a particular view, and since it pertains to the current MVC application and specific Views, what better place to hold this than the Models folder.  Obviously based on your needs, you could provide a flattened version of this ViewModel but it does provide a cleaner approach to separate the actual DTO of your entity from the surrounding auxiliary requirements for a view.

Note: you need to make sure that you always provide a default parameter-less constructor for your view models since the framework requires this on binding.

 

Summary

By using strongly-typed views, you get the best of all worlds. You are able to make use of the binding provided by the MVC framework and it also provides a clean approach to providing specific data for views. And if that still doesn't cut it for you, think that you're making a home out of your Models folder!

Strict use of strict mocks

3 Replies

Having a chat with a colleague the other, brought up the issue of strict mocks. I commented that strict mocks should be used sparingly and only in certain circumstance, and this led to a series of discussions on why that is.

Strict and loose Mocks

Usually mocking frameworks provide two types of mocks: strict and loose. If you’re using RhinoMocks, these are called StrictMock and DynamicMock. If you’re using Moq, you set the behaviour (Loose = Dynamic = Default or Strict) in the mock creation.

 

RhinoMocks

RhinoMock has a static mock repository that you can use to create mocks, called MockRepository. This saves you on having to explicitly create a mock repository. Thus, instead of doing:

 

   1: MockRepository mocks = new MockRepository();
   2:  
   3: IDependency dependency = mocks.DynamicMock<IDependency>();

you could write this:

 

   1: IDependency dependency = MockRepository.GenerateMock<IDependency>();

Using the static repository, you cannot pick between dynamic and strict mocks. All mocks are dynamic. This somehow promotes the usage of dynamic mocks over strict ones.

 

Moq

With Moq, as with RhinoMocks, the default behaviour is of loose mocks. Thus, writing:

 

   1: var dependency = new Mock<IDependency>();

is equivalent to:

 

   1: var dependency = new Mock<IDependency>(MockBehavior.Loose);

which in turn is the same as:

 

   1: var dependency = new Mock<IDependency>(MockBehavior.Default);

If you want to use strict mocks:

 

   1: var dependency = new Mock<IDependency>(MockBehavior.Strict);

(Note that with Moq, to access the actual Mocked object, you need to access the .Object property. RhinoMocks doesn’t require this because it uses extension methods).

 

Using Strict Mocks

So, now that we know how to create the two different types of mocks depending on the framework, let’s take a look at a SUT and see where the problem with strict mocks lie. To make the post shorter, I’ll use RhinoMocks, but you can accomplish the same with Moq.

Suppose we have the following class:

   1: public class ServiceClass 
   2:    {
   3:        private readonly IDependency _dependency;
   4:  
   5:        public ServiceClass(IDependency dependency)
   6:        {
   7:            _dependency = dependency;
   8:        }
   9:  
  10:        public void FirstMethod()
  11:        {
  12:            _dependency.FirstMethod();
  13:            Console.WriteLine("Called ServiceClass.FirstMethod");
  14:        }
  15:  
  16:        public void SecondMethod()
  17:        {
  18:            _dependency.SecondMethod();
  19:            Console.WriteLine("Called ServiceClass.SecondMethod");
  20:        }
  21:    }

 

and we want to test that when calling ServiceClass.FirstMethod, the corresponding call Dependency.FirstMethod is called, we could be tempted to do something like the following:

   1:        [Fact]
   2:        public void ServiceClass_FirstMethod_calls_dependency_FirstMethod_Strict()
   3:        {
   4:            MockRepository mocks = new MockRepository();
   5:  
   6:            IDependency dependency = mocks.StrictMock<IDependency>();
   7:            
   8:            using (mocks.Record())
   9:            {
  10:                dependency.Expect(m => m.FirstMethod()); 
  11:            }
  12:  
  13:            using (mocks.Playback())
  14:            {
  15:                ServiceClass serviceClass = new ServiceClass(dependency);
  16:  

 17:                serviceClass.FirstMethod();
  18:            }
  19:  
  20:            // Note that in the Record/Replay, VerifyAll is implicitly called
  21:        }

 

We’ve verified that Dependency.FirstMethod is called, everything passes, all is fine.

 

Welcome to BrittleVille

Two days later we get a change of request, that requires us to adjust the code for ServiceClass.FirstMethod. In particular, we need to call Dependency.ThirdMethod:

   1: public void FirstMethod()
   2: {
   3:     _dependency.FirstMethod();
   4:     _dependency.ThirdMethod();
   5:     Console.WriteLine("Called ServiceClass.FirstMethod");
   6: }

We check in the changes, run our tests, and boom! Test failed:

 

XunitException: Rhino.Mocks.Exceptions.ExpectationViolationException : IDependency.ThirdMethod(); Expected #0, Actual #1.

 

With strict mocks EVERY single call to the mocked object has to have expectations set on it. If we make an additional call and we don’t set the expectation, our test will fail. Our test was to verify the FirstMethod was being called, and it still is. Our test failed because we wrote it using strict mocks, it failed for the wrong reason, and made our code very brittle.

However, if we were to have written the test using Dynamic mocks, this wouldn’t happen:

   1: [Fact]
   2: public void ServiceClass_FirstMethod_calls_dependency_FirstMethod_Dynamic()
   3: {
   4:     MockRepository mocks = new MockRepository();
   5:  
   6:     IDependency dependency = mocks.DynamicMock<IDependency>();
   7:     
   8:     using (mocks.Record())
   9:     {
  10:         dependency.Expect(m => m.FirstMethod()); 
  11:     }
  12:  
  13:     using (mocks.Playback())
  14:     {
  15:         ServiceClass serviceClass = new ServiceClass(dependency);
  16:  
  17:         serviceClass.FirstMethod();
  18:     }
  19:  
  20:     // Note that in the Record/Replay, VerifyAll is implicitly called
  21: }

 

Note that out test is still valid. If we remove the call to Dependency.FirstMethod the test will fail as it should. However, it won’t fail if we add additional calls.

An added bonus to using loose mocks with RhinoMocks, is that you can use the AAA (Arrange-Act-Assert) syntax and also call individual expectations. Thus the previous code could be written like so:

   1: [Fact]
   2: public void ServiceClass_FirstMethod_calls_dependency_FirstMethod_AAA() {
   3:  
   4:     // Arrange
   5:     IDependency dependency = MockRepository.GenerateMock<IDependency>();
   6:  
   7:     dependency.Expect(m => m.FirstMethod());
   8:  
   9:  
  10:     // Act
  11:      ServiceClass serviceClass = new ServiceClass(dependency);
  12:  
  13:     serviceClass.FirstMethod();
  14:  
  15:     // Assert
  16:     dependency.VerifyAllExpectations();
  17:     
  18: }

 

Alternatively, if you don’t need expectations to return values, you can just call AssertWasCalled on them:

 

   1: [Fact]
   2: public void ServiceClass_FirstMethod_calls_dependency_FirstMethod_AAA() {
   3:  
   4:     // Arrange
   5:     IDependency dependency = MockRepository.GenerateMock<IDependency>();
   6:  
   7:     // Act
   8:      ServiceClass serviceClass = new ServiceClass(dependency);
   9:  
  10:     serviceClass.FirstMethod();
  11:  
  12:     // Assert
  13:     dependency.AssertWasCalled( m => m.FirstMethod());
  14:     
  15: }

 

Summary

Mocks are a great thing to test behaviour between different components. However, make sure you use them correctly if you don’t want to end up with brittle code that any minor change breaks a whole bunch of tests. Strict and ordered mocking should only be used in exceptional cases.

I’m not a Coding Horror, are you?

Leave a reply

According to you Jeff, 80% of developers are people that don't care much about improving the quality of their craft. In other words, it's mostly those who just care about the paycheck…programming is a job that pays the bills.

The remaining 20% fall in the category of those that read your blog (note the part in Italics):

But here's the paradox: the types of programmers who would most benefit from these guidelines, rules, principles, and checklists are the least likely to read and follow them. Throwing a book of rules at a terrible programmer just creates a terrible programmer with a bruise on their head where the book bounced off. This is something I discussed previously in Mort, Elvis, Einstein, and You:

Thus, if you read the article, you are most assuredly in the twenty percent category. The other eighty percent are not actively thinking about the craft of software development. They would never find that piece, much less read it. They simply don't read programming blogs– other than as the result of web searches to find quick-fix answers to a specific problem they're having. Nor have they read any of the books in my recommended reading list. The defining characteristic of the vast majority of these so-called "vocational" programmers is that they are unreachable. It doesn't matter what you, I or anyone else writes here — they'll never see it.

In the absence of mentoring and apprenticeship, the dissemination of better programming practices is often conveniently packaged into processes and methodologies. How many of these do you know? How many have you practiced?

 

 

Yet at the same time, as author of this blog and thus falling in the 20%, you state:

 

All those incredibly detailed rules, guidelines, methodologies, and principles? YAGNI. If it can't be explained on a single double-spaced sheet of paper, it's a waste of your time. Go read and write some code!

So which is it Jeff? Do we or don't we try and improve ourselves?

I don't fall into either of the groups, I'm not a Coding Horror. I'm one of those guys that tries to improve my knowledge, I try and learn continuously, either by myself or with the help of those that surround me. I don't just throw it all out of the window and claim that if there isn't a mentor around to help me, screw it. I don't give up if I can't understand it in a few hours. If others do differently, I try and understand why they do it differently. 

To claim that I'm not going to need to, or not change my ways is very arrogant on my behalf. And the best thing is that I'm not alone. There are thousands of others like me.

So please Jeff, don't corrupt the profession just to justify your own ways.

 

[And to my reader (Hi Mom!), I will do my utmost best to refrain from writing any more posts about Coding Horror]

Demoware Cowboys

2 Replies

My mother wants to become a computer programmer (not to be taken literally). And guess what, she can. With all the RAD environments nowadays you really don't need to know much about the basics of programming to make applications. You can use anything you want. First step, pick an environment. You have lots to choose from (Visual Studio, Delphi, Ruby on Rails). In under 1 hour you'll have your first application working. Next step, sell it.

Now do this for about 10 years, making a profit and you can claim that you're successful in software development. You can claim you've worked for 10 years and have never had to worry about things such as polymorphism, delegation, etc.

So when someone comes along with a post talking about weird things such as SOLID or how rules are meant to be broken, you reaffirm your position as a successful developer that has never needed or understood these concepts. And since the person writing this has 1.5M visitors a month and 125K subscribers, it's OK!

(You think I'm crazy. Read some of the comments on this post).

 

But It's NOT OK

I don't lose sleep over what I term demoware cowboys. Everyone has a right to make a living, however way they want. I'm the first person in the room that stands up to defend not needing to graduate in CS to have a successful career in software development. However, I also defend studying, understanding and practicing software engineering, and to continuously improve yourself. Just because you sell software and put food on the table doesn't mean you've accomplished all there is to in your field.

What does bother me however is people talking about things without truly understanding them. It's important to act with responsibility, specially when having so many followers.

Some of us write software for a living, as opposed to just writing about it,  and it's not only about getting the job done.

Additional code just for testing

Leave a reply

Take a look at this code:

  

   1: public class ClassToTest {

   2: 

   3:     public void MethodToTest()

   4:     {

   5:

   6:         IDAL dal = new DAL();

   7: 

   8:         int discount = dal.GetDiscount();

   9: 

  10:         if (discount > 10)

  11:         {

  12:             // Do something

  13:         }

  14: 

  15:     }

  16: }

The tight coupling to DAL just stands out like a sore thumb. One consequence of this tight coupling is that it's hard to test if we don't have an underlying database setup with correct values. Forget about testing this while taking a shower, unless you have a habit of taking your SQL Server box to the bath tub with you.

Take a look at the following approach to making this code testable without having a database: 

   1: public class ClassToTest {

   2: 

   3:     public void MethodToTest()

   4:     {

   5:

   6:         DAL dal = new DAL();

   7:

   8:         #if testing_in_shower

   9:         int discount = 3;

  10:         #else

  11:         int discount = dal.GetDiscount();

  12:         #endif

  13:         if (discount > 10)

  14:         {

  15:             // Do something

  16:         }

  17: 

  18:     }

  19: }

 

Are you still here? Didn't you just fall off of your chair, not knowing whether to laugh or cry in anguish? It's just plain wrong isn't it? Code Smell just takes on a whole new dimension (and consistency?). Something like this shouldn't be done for many reasons. It makes our code testable but it's wrong.

 

Introducing code for testing purposes

The reason I bring this up now is because in the past 2 weeks I've seen a few references and examples of introducing code just for the sake of testability. Two weeks ago I was skimming over an article that was explaining unit tests. The class the author was trying to test had a dependency that was causing issues during his tests. The solution offered was to introduce a new constructor and inject the dependency in. In our case, the ClassToTest would like:

  

   1: public class ClassToTest {

   2: 

   3:     public ClassToTest()

   4:     {

   5:

   6:         // Initialize some stuff

   7: 

   8:     }

   9: 

  10:     public ClassToTest(IDAL dal)

  11:     {

  12:         // Initialize some stuff

  13:         _dal = dal;

  14:     }

  15: 

  16: ....

Now that's great. The problem is that the author left both constructors in there, emphasizing that one is for testing purposes and the other for production code. Why that's wrong I'll get to in a moment.

The other day, Scott Hansleman made a blog post on testing some code that had a dependency on the current principal. His solution was to inject IPrincipal into various methods of the Controller, where Controller is in the context of an ASP.NET MVC Controller. Personally I don't agree with the solution but that's another subject (and another post on a different approach I have in the backlog for the same scenario). However, injecting the dependency seemed valid enough. Later on he updated the post with the following:

 

UPDATE: Phil had an interesting idea. He said, why not make method overloads, one for testing and one for without. I can see how this might be controversial, but it's very pragmatic.

  1. [Authorize] 
  2. public ActionResult Edit(int id) 
  3. { 
  4. return Edit(id, User); //This one uses HttpContext
  5. }  
	[Authorize]
	public ActionResult Edit(int id)
	{
	return Edit(id, User); //This one uses HttpContext
	}

You'd use this one as before at runtime, and call the overload that takes the IPrincipal explicitly for testing.

The same thing has happened as in the case of an overloaded constructor. Code has been introduced into the base just for the sake of testability. In the previous case it was a new constructor, and in this case, a new method.

 

Why is this wrong?

Introducing code just so that you can test it is wrong for several reasons, including an increase in noise, decrease in readability and more importantly, permitting the possibility of production code that will not be tested. It might seem improbable at first, seeing that the overloaded versions always call the base ones, but as your code base grows, by allowing this habit, you can promote it to other structures such as conditionals, switch statements, etc, and not limit it only to methods or constructors. Then you'll start initializing values that only make sense in tests and gradually your code starts to smell more and more. To top it off, you can throw in some compiler conditional that rips those parts of the code out from the production version.

Code should also serve as documentation. The cleaner you make your code, the less noise and redundancy you add, the easier it is for others to understand and maintain.

 

Conclusion

Being pragmatic is good, but don't do it at any cost. Try and not introduce code into your code base that is there exclusively for the purpose of testability (and no, dependency injection is not only for testability). Anything that has to do with testing, keep it where it should be, in the test assemblies. Having overloaded methods or constructors, mocks or stubs in your code base is just as bad as having compiler conditionals in your code for testability. If you run into issue where your code is hard to test, step back and re-think your design. Maybe the problem you're facing now that's impeding your tests is actually a can of worms waiting to be opened. And the sooner you open it, the better. That's one of the great advantages of Test Driven Development.

Is it time to rename TDD?

5 Replies

Just reading a comment on Uncle Bob’s blog by a guy called Ted.

What’s with this whole TDD/SOLID and Podcast stuff?

Just because you practiced TDD does not make your code clean nor high-quality.

Let’s face it, most TDD practitioners are “DEVELOPERS”. You are not “QA”, “TESTER”, “SDET”. What do you know about various testing techniques, testing principles, testing metrics, how to test, etc?

    Do our field a favor TDD-er, do the following:
  1. How We Test Software at Microsoft
  2. Go to MS and apply for SDET position (or Google)
  3. Read more books about Software Testing
  4. Ask yourself if you wrote great TDD code or not

Now for the last question, if the answer is “Yes”, then you’re a Tester. If the answer is “No”, then you’re a developer. And obviously, if you are a developer, you suck at TDD.

It’s just how the brain is wired or perhaps it’s in your training. Developers are almost always bad testers, vice versa.

Maybe TDD should be renamed "Coming up with a good design by first trying to use my code Driven Development". There is an overwhelming amount of information regarding Test Driven Development and yet, still, STILL, people think it’s about testing.