ASP.NET MVC2 In Action

I started working through ASP MVC2 In Action

 

A couple of nit-pciks

1) They abuse the var keyword and don’t follow best practices in terms of variable naming.  For example, on page 18:

            var model = new GuestBookEntry();

            return View(model)

 

Using var here is just lazy and using the variable “model” leads to confusion– it should be:

            GuestBookEntry guestBookEntry = new GuestBookEntry();

            return View(guestBookEntry);

 

Because that is what the controller is actually returning

2) They don’t fully-qualify the model on the views.  For example, on page 18 again:

<%@ Page Title=""

    Language="C#"

    MasterPageFile="~/Views/Shared/Site.Master"

    Inherits="System.Web.Mvc.ViewPage<GuestBookEntry>" %>

 

It should be

<%@ Page Title=""

    Language="C#"

    MasterPageFile="~/Views/Shared/Site.Master"

    Inherits="System.Web.Mvc.ViewPage<Com.Tff.GuestBook.Models.GuestBookEntry>" %>

 

I don’t know any way to NOT use the namespace using MVC (they must, unless their book code doesn’t compile OR they are putting everything in the same namespace – Ugh!)

I DO like the fact that they put line breaks in the View definition – it is much more readable.

Aside from the nit-picks, I have 1 larger beef.

I believe that having more models (DTOs, POCOs, whatever you want to call them) is better than have 1 uber-model that you shoe horn into all situations.  In fact, having a use-case specific model follows the single-responsibility principle very well.  I DON’T agree with the use of nested classes.  For example, on page 29 they recommend (constructor is mine – I put it in the get the View to match their screen shots)

    public class CustomerSummary

    {

public CustomerSummary(string name, string serviceLevel, string orderCount, string mostRecentOrderDate, bool active)

        {

            this.Name = name;

            this.ServiceLevel = serviceLevel;

            this.OrderCount = orderCount;

            this.MostRecentOrderDate = mostRecentOrderDate;

            this.Input = new CustomerSummaryInput();

            this.Input.Active = active;

        }

 

        public string Name { get; set; }

        public string Active { get; set; }

        public string ServiceLevel { get; set; }

        public string OrderCount { get; set; }

        public string MostRecentOrderDate { get; set; }

        public CustomerSummaryInput Input { get; set; }

 

    }

 

    public class CustomerSummaryInput

    {

        public int Number { get; set; }

        public bool Active { get; set; }

    }

 

Yuck!

There should be a CustomerSummary class and a CustomerInput class.  If you need to link the two – that is what primary keys are for (heck, they start that with the Number property in CustomerSummaryInput.  This nested class combines 2 different things (displaying the customer and getting new customer info).  If there needs to be a customer base class (FirstName, LastName, Active) and then derived classes with the ServiceLevel, etc…, that is much more cleaner.