ASP.NET MVC – Is it a Pattern Smell to Put Getters in Your Model?

asp.net-mvccdesign-patterns

I'm working on an ASP.NET MVC app, and I've been getting into the habit of putting what seem like helpful and convenient getters into my model/entity classes.

For example:

public class Member
{
    public int Id { get; set; }
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string PhoneNumber { get; set; }

    public string FullName
    {
        get { return FirstName + " " + LastName; }
    }

    public string FormattedPhoneNumber
    {
        get { return "(" + PhoneNumber.Substring(0, 3) + ") " + PhoneNumber.Substring(3, 3) + "-" + PhoneNumber.Substring(6); }
    }
}

I'm wondering people think about the FullName and FormattedPhoneNumber getters.

They make it very easy to create standardized data formats throughout the app, and they seem to save a lot of repeated code, but it could definitely be argued that data format is something that should be handled in mapping from model to view-model.

In fact, I was originally applying these data formats in my service layer where I do my mapping, but it was becoming a burden to constantly have to write formatters then apply them in many different places. E.g., I use "Full Name" in most views, and having to type something like model.FullName = MappingUtilities.GetFullName(entity.FirstName, entity.LastName); all over the place seemed a lot less elegant than just typing model.FullName = entity.FullName (or, if you use something like AutoMapper, potentially not typing anything at all).

So, where do you draw the line when it comes to data formatting. Is it "okay" to do data formatting in your model or is that a "pattern smell"?

Note: I definitely do not have any html in my model. I use html helpers for that. I'm strictly talking about formatting or combining data (and especially data that is frequently used).

Best Answer

In your example, I like the FullName getter (for all the reasons you have given) but I do not like the FormattedPhoneNumber getter. The reason is: it's probably not that easy (once you have international phone numbers etc.) and if you place the logic for formating phone numbers in a method of Member, chances are you will need to refactor (or copy-paste caugh) once you need a formated phone number for Institution, Vendor etc. too.

EDIT: IMO it would be better to have a PhoneNumber class with a Formatted getter.

Related Solutions

C# class architecture for REST services

Why not make it easy on your self - both decision-wise and implementation-wise - and just use one of the many frameworks that already exist to consume Web API's, like RestSharp or Hammock.

ASP.NET MVC – Separating Data Access in ASP.NET MVC

Generally, you want your Controllers to do only a few things:

Handle the incoming request
Delegate the processing to some business object
Pass the result of the business processing to the appropriate view for rendering

There shouldn't be any data access or complex business logic in the controller.

[In the simplest of apps, you can probably get away with basic data CRUD actions in your controller, but once you start adding in more than simple Get and Update calls, you are going to want to break out your processing into a separate class.]

Your controllers will usually depend on a 'Service' to do the actual processing work. In your service class you may work directly with your data source (in your case, the DbContext), but once again, if you find yourself writing a lot of business rules in addition to the data access, you will probably want to separate your business logic from your data access.

At that point, you will probably have a class that does nothing but the data access. Sometimes this is called a Repository, but it doesn't really matter what the name is. The point is that all of the code for getting data into and out of the database is in one place.

For every MVC project I've worked on, I've always ended up with a structure like:

Controller

public class BookController : Controller
{
    ILibraryService _libraryService;

    public BookController(ILibraryService libraryService)
    {
        _libraryService = libraryService;
    }

    public ActionResult Details(String isbn)
    {
        Book currentBook = _libraryService.RetrieveBookByISBN(isbn);
        return View(ConvertToBookViewModel(currentBook));
    }

    public ActionResult DoSomethingComplexWithBook(ComplexBookActionRequest request)
    {
        var responseViewModel = _libraryService.ProcessTheComplexStuff(request);
        return View(responseViewModel);
    }
}

Business Service

public class LibraryService : ILibraryService
{
     IBookRepository _bookRepository;
     ICustomerRepository _customerRepository;

     public LibraryService(IBookRepository bookRepository, 
                           ICustomerRepository _customerRepository )
     {
          _bookRepository = bookRepository;
          _customerRepository = customerRepository;
     }

     public Book RetrieveBookByISBN(string isbn)
     {
          return _bookRepository.GetBookByISBN(isbn);
     }

     public ComplexBookActionResult ProcessTheComplexStuff(ComplexBookActionRequest request)
     {
          // Possibly some business logic here

          Book book = _bookRepository.GetBookByISBN(request.Isbn);
          Customer customer = _customerRepository.GetCustomerById(request.CustomerId);

          // Probably more business logic here

          _libraryRepository.Save(book);

          return complexBusinessActionResult;

     } 
}

Repository

public class BookRepository : IBookRepository
{
     LibraryDBContext _db = new LibraryDBContext();

     public Book GetBookByIsbn(string isbn)
     {
         return _db.Books.Single(b => b.ISBN == isbn);
     }

     // And the rest of the data access
}

Best Answer

Related Solutions

C# class architecture for REST services

ASP.NET MVC – Separating Data Access in ASP.NET MVC

Related Topic