A more structured MembershipProvider.

If you take a look at the official documentation that tells you how to create your own custom MembershipProvider you’ll notice that the documentation is mostly about describing:

  • how you should throw exceptions and when
  • Which events you should invoke and when
  • How the logic in each method should work.

Let’s stop here for a moment. What are the most typical reason for creating a custom membership provider? Is it to change the behaviour(/contract) of the provider? Or maybe change the password handling? No. The most common reason is to add support for a custom data source.

Back to the MembershipProvider class. As I see it, it has four responsibilites:

  1. Be able to handle all members (login, register etc)
  2. Load/Store information from/in a data source
  3. Keep track of account/password policies and handle those.
  4. Handle passwords (encrypt/hash/compare)

And that’s why it’s such a tedious task to create a custom MembershipProvider: You need to implement all of those responsibilities.

Griffin.MvcContrib to the rescue!

With the MembershipProvider in Griffin.MvcContrib you do not need to know all of those details. Our MembershipProvider uses DependencyResolver (a built in
Service Locator in MVC3) to lookup the dependencies that it needs. You need to register the following services in your inversion of control container:

IAccountRepository

Used to retrieve/store membership accounts from/to a data source. It’s the only thing you need to implement to get support for a custom data source (like a database or a web service).

Source code

IPasswordStrategy

Used to handle passwords (comparing them, encrypting, decrypting). There are three strategies available in the framework:

  • ClearTextStrategy – Store passwords as clear text in the database
  • HashedPasswordStrategy – Hash passwords before they are stored.
  • TripleDesPasswordStrategy – Use TripleDes to encrypt/decrypt passwords.

Source code

IPasswordPolicy

Defines how passwords should be treated. For instance the minimum length, number of attempts before locking the account etc.

Two policy implementations exist. One who loads the policies from web.config and one “normal” class which you can create and assign the properties in.

Source code

How to use it

Disclaimer: You should know that Account Administration Tools won’t work since the provider has a dependency on DependencyResolver. Other than that, the provider works like any other provider.

Start by modifying your web.config to specify the new provider:

    <membership defaultProvider="GriffinMembershipProvider">
      <providers>
        <clear />
        <add name="GriffinMembershipProvider" type="Griffin.MvcContrib.Providers.Membership.MembershipProvider" applicationName="/" />
      </providers>
    </membership>

Then you need to register the dependencies in your inversion of control container. Here is a sample for Autofac:

protected void Application_Start()
{
	ContainerBuilder builder = new ContainerBuilder();
	builder.RegisterType<HashPasswordStrategy>().AsImplementedInterfaces();
	builder.RegisterType<RavenDbAccountRepository>().AsImplementedInterfaces();
	builder.RegisterInstance(new PasswordPolicy
		 {
			 IsPasswordQuestionRequired = false,
			 IsPasswordResetEnabled = true,
			 IsPasswordRetrievalEnabled = false,
			 MaxInvalidPasswordAttempts = 5,
			 MinRequiredNonAlphanumericCharacters = 0,
			 PasswordAttemptWindow = 10,
			 PasswordMinimumLength = 6,
			 PasswordStrengthRegularExpression = null
		 }).AsImplementedInterfaces();
								 
	// the rest of the initialization
}

If you like to switch from hashed passwords to encrypted passwords you just need to register TripleDesStrategy in the container instead. Or implement your own strategy.

Summary

The code is available at github and can be installed using nuget install-package griffin.mvccontrib.


Why extension methods should not be used as part of a framework.

Two of my favorite frameworks/libraries uses extension methods heavily. Autofac uses extension methods during registration and ASP.NET MVC3 for their HTML Helpers. Since extension methods are static, they are closed for extension. Which are one of the most important principles for object oriented programming.

For MVC I would like to be able to add a tooltip automatically for all editors and labels. It’s really simple, I just want to add a “title” attribute to all generated HTML elements where the ModelMetadata contains a “Description”. The source code for LabelFor looks like this:

    public static class LabelExtensions {
        public static MvcHtmlString Label(this HtmlHelper html, string expression) {
            return LabelHelper(html,
                               ModelMetadata.FromStringExpression(expression, html.ViewData),
                               expression);
        }

        [SuppressMessage("Microsoft.Design", "CA1006:DoNotNestGenericTypesInMemberSignatures", Justification = "This is an appropriate nesting of generic types")]
        public static MvcHtmlString LabelFor<TModel, TValue>(this HtmlHelper<TModel> html, Expression<Func<TModel, TValue>> expression) {
            return LabelHelper(html, 
                               ModelMetadata.FromLambdaExpression(expression, html.ViewData), 
                               ExpressionHelper.GetExpressionText(expression));
        }

        public static MvcHtmlString LabelForModel(this HtmlHelper html) {
            return LabelHelper(html, html.ViewData.ModelMetadata, String.Empty);
        }

        internal static MvcHtmlString LabelHelper(HtmlHelper html, ModelMetadata metadata, string htmlFieldName) {
            string labelText = metadata.DisplayName ?? metadata.PropertyName ?? htmlFieldName.Split('.').Last();
            if (String.IsNullOrEmpty(labelText)) {
                return MvcHtmlString.Empty;
            }

            TagBuilder tag = new TagBuilder("label");
            tag.Attributes.Add("for", html.ViewContext.ViewData.TemplateInfo.GetFullHtmlFieldId(htmlFieldName));
            tag.SetInnerText(labelText);
            return tag.ToMvcHtmlString(TagRenderMode.Normal);
        }
    }

Hence it’s impossible to do anything about the helper. I do understand why they use extension methods. It’s great since it’s very easy to find all available helpers. The problem is that you can’t do anything do modify the data that they generate.

The solution

Convert all extension methods to facades for the real implementation, and use DependencyResolver in the extension methods to find the proper classes. Create some interfaces and default implementations for all generators.

The new LabelHelper:

internal static MvcHtmlString LabelHelper(HtmlHelper html, ModelMetadata metadata, string htmlFieldName)
{

    var labelGenerator = DependencyResolver.Resolve<ILabelGenerator>();
    return labelGenerator.Create(html, metadata, htmlFieldName);
}

The default implementation (simplified example):

	public class LabelGenerator : ILabelGenerator
	{
	
		public virtual void Create(HtmlHelper html, ModelMetadata metadata, string htmlFieldName)
		{
            string labelText = metadata.DisplayName ?? metadata.PropertyName ?? htmlFieldName.Split('.').Last();
            if (String.IsNullOrEmpty(labelText)) {
                return MvcHtmlString.Empty;
            }

            TagBuilder tag = new TagBuilder("label");
            tag.Attributes.Add("for", html.ViewContext.ViewData.TemplateInfo.GetFullHtmlFieldId(htmlFieldName));
            tag.SetInnerText(labelText);
			ProcessTag(tag);
            return tag.ToMvcHtmlString(TagRenderMode.Normal);
		}
			
		protected virtual void ProcessTag(TagBuilder tag, ModelMetaData metaData)
		{
		}
	}

Which would allow me to create the following implementation:

	public class LabelWithTitlesGenerator : LabelGenerator
	{
		protected override void ProcessTag(TagBuilder tag, ModelMetaData metaData)
		{
			if (metaData.Description != null)
				tag.Attributes.Add("title", metaData.Description);
		}
	}

The only thing left is to inject it in my container and create a small jquery script which creates the actual tooltip using the “title” attribute.

Summary

Extension methods are closed for extension. Once declared you can not do anything to extend the functionality that they provide. It’s the same as if you would develop a framework where you put sealed on all your classes.

Extension methods were created to be able to make life easier by proving convenience methods for existing classes. The intention was not that you should use them as a part of your framework since you close down that part for extension if you do. The extension methods become locking out methods ;O


Easy model and validation localization in ASP.NET MVC3

Update I’ve written a more complete article about the framework at codeproject.com.


If you have googled a bit you’ll read that you should use DescriptionAttribute and DisplayNameAttribute to get localization which will result in a view model that looks something like this:

    public class UserViewModel
    {
        [Required(ErrorMessageResourceName = "Required", ErrorMessageResourceType = typeof(Resources.LocalizedStrings))]
        [LocalizedDisplayName(ErrorMessageResourceName = "UserId", ErrorMessageResourceType = typeof(Resources.LocalizedStrings))]
        [LocalizedDescription(ErrorMessageResourceName = "UserIdDescription", ErrorMessageResourceType = typeof(Resources.LocalizedStrings))]
        public int Id { get; set; }

        [Required(ErrorMessageResourceName = "Required", ErrorMessageResourceType = typeof(Resources.LocalizedStrings))]
        [LocalizedDisplayName(ErrorMessageResourceName = "UserFirstName", ErrorMessageResourceType = typeof(Resources.LocalizedStrings))]
        [LocalizedDescription(ErrorMessageResourceName = "UserFirstNameDescription", ErrorMessageResourceType = typeof(Resources.LocalizedStrings))]
        public string FirstName { get; set; }

        [Required(ErrorMessageResourceName = "Required", ErrorMessageResourceType = typeof(Resources.LocalizedStrings))]
        [LocalizedDisplayName(ErrorMessageResourceName = "UserLastName", ErrorMessageResourceType = typeof(Resources.LocalizedStrings))]
        [LocalizedDescription(ErrorMessageResourceName = "UserLastNameDescription", ErrorMessageResourceType = typeof(Resources.LocalizedStrings))]
        public string LastName { get; set; }
    }

Don’t do that.

The solution to get cleaner DataAnnotation localization is to derive the attributes and do some magic in the derived classes. The problem with that solution is that client-side validation stops working unless you create new adapters for your derived attributes.

Don’t do that.

The easy solution

I found an excellent post by Brad Wilson that goes through most of the new stuff in MVC3. It’s a must read. When reading it I came up with the idea to use the new Meta Data providers to do the localization. The solution works quite well and you’re models become clean again:

    public class UserViewModel
    {
        [Required]
        public int Id { get; set; }

        [Required]
        public string FirstName { get; set; }

        [Required]
        public string LastName { get; set; }
    }

You need to specify the new providers (created by me) in your global.asax:

        protected void Application_Start()
        {
            var stringProvider = new ResourceStringProvider(Resources.LocalizedStrings.ResourceManager);
            ModelMetadataProviders.Current = new LocalizedModelMetadataProvider(stringProvider);
            ModelValidatorProviders.Providers.Clear();
            ModelValidatorProviders.Providers.Add(new LocalizedModelValidatorProvider(stringProvider));
        }

That’s it. No more messy attributes.

Defining the localization resources

I created an interface called ILocalizedStringProvider which is used by both providers. This means that you do not have to use String Tables. Just create a new class and implement that interface to get support for your database, XML file or whatever you choose.

In my example I used a string table and the strings in it looks like this:

As you see, you should name your strings as “ModelName_PropertyName” and “ModelName_PropertyName_MetadataName” to include meta data like Watermark and NullDisplayText. Refer to MSDN to see which kind of meta data there is.

As for validation attributes, the string table names should simply be the attribute name without the “Attribute” suffix.

Code and documentation

The code is available at github.
Documentation is available here.

Update

Comments are now closed. Either open tickets at github or ask questions at stackoverflow.com with the tag “griffin.mvccontrib”. Write a comment starting with “@jgauffin” if I don’t answer within reasonable time ;)


Getting information into the Layout without using ViewBag.

You should avoid using the ViewBag as much as possible since you get run-time errors instead of compile time error if something fails. The problem is that it can be hard to avoid it if you are using information in your layout.

The solution that I’m using lets you do the following:

public class YourController : BaseController
{
    public ActionResult YourAction()
    {
        LayoutModel.MetaDescription = "Does something in the application";
        return View();
    }
}

And in your _Layout.cshtml:

<meta name="description" value="@LayoutModel.MetaDescription" />

See? Everything is typed.

The approach I’m using is to create a base class for views and inherit WebViewPage in it. First things first, add a model for your layout:

public class LayoutViewModel
{
    public string MetaKeywords { get; set; }
}

Then add the following to your BaseController:

public class BaseController : Controller
{
    private LayoutViewModel _layoutModel = new LayoutViewModel();

    public LayoutViewModel LayoutModel { get { return _layoutModel; } }

    protected override void OnResultExecuting(ResultExecutingContext ctx) 
    {
        var viewResult = ctx.Result as ViewResult;
        if (viewResult != null)
            viewResult.ViewBag.LayoutModel = LayoutModel;

        base.OnResultExecuting(ctx);
    }
}

Ok. I know. I’m cheating and using the ViewBag. But since it’s only in one place it’s less error prone than using the viewbag everywhere.

Now create the View base class. You need to create two versions to have support for typed views.

public abstract class ViewBaseWithLayoutModel : WebViewPage
{
    public LayoutViewModel LayoutModel { get { return (LayoutViewModel)ViewBag.LayoutModel; } }
}
public abstract class ViewBaseWithLayoutModel<T> : WebViewPage<T>
{
    public LayoutViewModel LayoutModel { get { return (LayoutViewModel)ViewBag.LayoutModel; } }
}

The last thing is to specify your new base view class in your ViewsWeb.config:

  <system.web.webPages.razor>
    <pages pageBaseType="Namespace.To.ViewBaseWithLayoutModel">

That’s it.


Begin/End asynchronous methods vs the new Async methods

A new asynchronous model was introduced in .NET 3.5sp1 where all methods ends with ‘Async’ like Socket.RecieveAsync. I will not give you any code samples, there are several articles out there which shows how the new model work. Instead I’m going to try help you understand the differences and to select which model you should use for your project.

The benefit with the new model is that it’s a lot more similar to the I/O Completion Ports threading model which exists in Windows (and is used internally for all asynchronous operations in .NET). The idea is that the IO model should create as few objects as possible per operation, compared to the old model where one IAsyncResult object is created per operation.

I haven’t looked at the code (using reflector) for the old model versus the new one, but I bet that the code for the new model is a lot more efficient than the old one since it’s more similar to IOCP.

Which method should you choose then?

Choose the one that you are most comfortable with (which probably is Begin/End since it’s the model that have existed since day one). It’s more likely that you loose performance instead of gaining
if you pick a model that you do not fully understand.

As always: Never optimize something unless it have been proven to be a problem.


An easier way to debug windows services

Have you got tired of attaching the Visual Studio debugger to the service application? I got the solution just for you! It’s a small helper class containing a static method which you need to invoke.

public static void Main(string[] argv)
{
    // just include this check, "Service1" is the name of your service class.
    if (WindowsServiceHelper.RunAsConsoleIfRequested<Service1>())
        return;

    // all other code 
}

Then go to project properties, the “Debug” tab and add “-console” as Command Arguments.

Shows the debug settings under project properties

How to configure Visual Studio

That’s it. What I do is simply to allocate a console using the winapi and then invoke (through reflection) the proper protected methods in your service class.

Source code for the helper class:

public static class WindowsServiceHelper
{
    [DllImport("kernel32")]
    static extern bool AllocConsole();

    public static bool RunAsConsoleIfRequested<T>() where T : ServiceBase, new()
    {
        if (!Environment.CommandLine.Contains("-console"))
            return false;

        var args = Environment.GetCommandLineArgs().Where(name => name != "-console").ToArray();

        AllocConsole();

        var service = new T();
        var onstart = service.GetType().GetMethod("OnStart", BindingFlags.Instance | BindingFlags.NonPublic);
        onstart.Invoke(service, new object[] {args});

        Console.WriteLine("Your service named '" + service.GetType().FullName + "' is up and running.\r\nPress 'ENTER' to stop it.");
        Console.ReadLine();

        var onstop = service.GetType().GetMethod("OnStop", BindingFlags.Instance | BindingFlags.NonPublic);
        onstop.Invoke(service, null);
        return true;
    }
}

Inversion of control containers – Best practices

Disclaimer: I’ve used IoC containers for a couple of years now and also made a couple of my own (just to learn, nothing released). I’ve also consumed loads of documentation in the form of blogs and stackoverflow questions. And I thought that I should share what I’ve learned. In other words: These are my very own subjective best practices.

An IoC container is not a Service Locator

One of the first mistakes that you’ll make is to register the container in the container or create a singleton to get access to it. It basically means that you can do something like:

public class MyService
{
    MyService(IServiceResolver myContainer)
    {
    }

    public void DoSomething()
    {
        var service = _container.Resolve<ISomeService>();
        service.DoMore();
    }
}

Do not do that. Service location is another pattern that you get as a “bonus” when you use a IoC container. Some even call Service Location as an anti-pattern. Google it.

The most important reasons is that you hide a dependency that you won’t discover until you forget to register IAnotherService. What if the method is only used during edge cases? Then the dependency wont be discovered until that edge case is fulfilled.

Using the constructor to inject dependencies will also help you discover violations to Single Responsibility principle. Having a lot of dependencies might suggest that you need to break your class into several smaller classes.

There are some cases when you THINK that you really need to use the container in your class. Example:

    public class YourService
    {
        public YourService(IContainer container)
        {
        }

        public void YourMethod()
        {
            var user = _container.Get<IUser>();
        }
    }

The proper solution is to register a factory in the container:

    public class YourService
    {
        public YourService(IDomainModelFactory factory)
        {
        }

        public void YourMethod()
        {
            var user = _factory.Create<IUser>();
        }
    }

The smaller interface, the better

Interfaces are not classes. There is nothing saying that there should be a 1-1 mapping between them. imho it’s far better to create several small interfaces and let the same class implement them than creating a larger one having all all features that the class have.

Do design the interfaces before you design the classes. The reason is that if you do the opposite you’ll usually end up with one interface per class.

Smaller interfaces also makes it easier to let your application grow since you can move smaller parts of the implementation (as in creating a new class for one of the interfaces).

Don’t choose a longer lifetime to get better performance

This is a mistake that I’ve done dozens of times. Object creation will usually not be a problem when you are running your application.

The advantage with short lifetimes is that it get easier to handle context sensitive dependencies such as a database connections.

Using a longer lifetime (which lives longer than the mentioned context sensitive dependencies) usually means that you create same kind of service location implementation or custom factories.

Hence you get code like this:

public class UserService : IUserService
{
    public void Add()
    {
        var (var uow = UnitOfWork.create())
        {
            // do stuff.
            uow.Save();
        }
    }
}

instead of

public class UserService : IUserService
{

    public UserService(IUnitOfWork uow)
    {

    }

    public void Add()
    {
        // do stuff with the unit of work here.
    }
}

what’s the problem with the first approach? Well. It’s usually the CALLER that knows what should be done in a unit of work, not the service itself. I’ve seen people create complex Unit Of Work solutions to get round that problem. The other simpler approach is to use TransactionScope.

Both of those solutions are either slower or more complex than services with shorter lifetime. Save the optimizations to when object instantiation have been proven to be a problem.

Don’t mix lifetimes

Mixing lifetimes can lead to undesired effects if you are not careful. Especially when projects have begun to grow and getting more complex dependency graphs.

Example

// EF4 context, scoped lifetime
class Dbcontext : IDbContext
{
}

// short lifetime, takes db context as a dependency
class Service1 : IService1
{
}

// Single instance, keeps a cache etc.
class Service2 : IService2
{
}

Service2 takes service1 as a dependency which in turn requires a dbcontext. Let’s say that the dbcontext has an idle timeout which closes the connection after a while. That would make service1 fail and also service2 which depends on it.

Simply be careful when using different lifetimes.

Try to avoid primitives as constructor parameters

Try to avoid using primitives as constructor parameters (for instance a string). It’s better to take in object since it makes extension (subclassing) easier.

Hey, I need my primitives you say. Take this example:

public class MyRepository
{
    public MyRepository(string connectionString)
    {
    }
}

Well. You’re class breaks SRP. It acts as a connection factory and a repository. Break the factory part out:

public class MyRepository
{
    public MyRepository(IConnectionFactory factory)
    {
    }
}

Avoid named dependencies

Named dependencies are as bad as magic strings. Don’t use them.

If you need different implementations simply create different interfaces such as IAccountDataSource and IBillingDataSource instead of just taking in IDataSource.

imho this is a possible violation of Liskovs Substitution principle.


Making custom dataannotation attributes work with MVC client side validation

In a previous blog entry I showed you a easier approach to localized DataAnnotation attributes.

The problem with that approach was that client side validation didn’t work. It isn’t hard to fix. All you need is to borrow some code from the built in DataAnnotation support and reference the custom attributes instead.

Here is a sample implementation.

    public class RequiredAttributeAdapter : DataAnnotationsModelValidator<RequiredAttribute>
    {
        public RequiredAttributeAdapter(ModelMetadata metadata, ControllerContext context, RequiredAttribute attribute)
            : base(metadata, context, attribute)
        {
        }

        public static void SelfRegister()
        {
            DataAnnotationsModelValidatorProvider
                .RegisterAdapter(
                    typeof (RequiredAttribute),
                    typeof (RequiredAttributeAdapter));
        }

        public override IEnumerable<ModelClientValidationRule> GetClientValidationRules()
        {
            return new[] { new ModelClientValidationRequiredRule(ErrorMessage) };
        }
    }

Then simply call RequiredAttributeAdapter.SelfRegister(); somewhere in your Global.asax


Handling JSON responses in jquery plugins

I’ve been coding several jquery plugins lately and most of my plugins uses Ajax and JSON to handle data. Each plugin gets a specific JSON string returned that must follow a specific format. For instance, the table plugin expects to get total number of rows and an array containing the rows to add:

	var jsonResponse = { 
		"TotalRowCount": 2,
		"Debug": true,
		"TableMode": 'Append', 
		"Rows" : [
			{ "Id": 1, "Title": "Fool", "FirstName": "Jonas", "LastName": "Gauffin", "Age": 34, "Options": "<a href="/add">aa</a>" },
			{ "Id": 2, "FirstName": "Arne", "LastName": "Purka", "Age": 14},
		]
	};

Nothing strange with that. The problem is when I want to return something else than rows. The plugin cannot handle that. I might for instance want to return an error message informing the user that something went wrong.

My first attempt was to change so that all plugins used a structure like this:

var response = {
    "ContentType": 'Rows',
    "Body": {
      "TotalRowCount": 2,
      "Debug": true,
      "TableMode": 'Append',
      "Rows" : [
        { "Id": 1, "Title": "Fool", "FirstName": "Jonas", "LastName": "Gauffin", "Age": 34, "Options": "<a href="/add">aa</a>" },
        { "Id": 2, "FirstName": "Arne", "LastName": "Purka", "Age": 14},
      ]
    }
}

where the Body data would be everything in the first JSON code sample. But since javascript is dynamic I felt that it’s really not necessary to wrap everything like that. I just add the ContentType to all original JSON responses.

	var jsonResponse = { 
		"ContentType": 'Rows',
		"TotalRowCount": 2,
		"Debug": true,
		"TableMode": 'Append', 
		"Rows" : [
			{ "Id": 1, "Title": "Fool", "FirstName": "Jonas", "LastName": "Gauffin", "Age": 34, "Options": "<a href="/add">aa</a>" },
			{ "Id": 2, "FirstName": "Arne", "LastName": "Purka", "Age": 14},
		]
	};

In this way I only have to check the ContentType before doing anything else and then take an appropriate action.

Right now I’m trying to figure out if I should add a general JSON response pre processor which is invoked before the actual plugin get’s to handle the response. (declare it in my own jquery plugin namespace and invoke it from each plugin if it exist). In that way I can move all general handling (like handling errors) to a separate place instead having to duplicate it in all plugins.


Single Responsibility Prinicple

SRP is one of the easiest principles to learn, but one of the hardest to master. The reason to this is that it can be quite hard to see if a method or class breaks SRP or not. I got a few simple rules that will help you check if you are breaking the principle.

First of all, lets check the definition of SRP:

every object should have a single responsibility, and that responsibility should be entirely encapsulated by the class

Hence my interpretation varies a bit from the original statement, as I like to apply it to classes, methods and interfaces. If you do not agree: Go buy an ice-cream, eat it, put a smile on your face and continue reading.

XmlDoc is an excellent tool if you use it correctly. So many users abuse it and screams out to the world their product/library/etc has DOCUMENTATION! The DoXZ! yay! What they fail to mention is that the documentation is simply a reflection of the methods and classes. Read my other blog posts to get a few tips on how to write good documentation. Back to SRP. Documentation can be a tool for SRP too. If you find yourself using AND’s when describing a method, class or an method argument you have most likely broken SRP. Refactor.

When talking about SRP it’s important to know what is meant with single responsibility. When it comes to methods, they should only contain logic for one type of responsibility. Here is a small example:

public class UserService
{
     public void Register(string email, string password)
     {
          if (!email.Contains("@"))
              throw new ValidationException("Email is not an email!");

         var user = new User(email, password);
         _database.Save(user);

         _smtpClient.Send(new MailMessage("mysite@nowhere.com", email){Subject="HEllo fool!"});
     }
}

The name Register suggests that the method should register the user in the system. Doing email validation doesn’t seem to belong in an register method. Let’s break it out into a new method.

public class UserService
{
     public void Register(string email, string password)
     {
          if (!ValidateEmail(email))
              throw new ValidationException("Email is not an email!");

         var user = new User(email, password);
         _database.Save(user);

         _smtpClient.Send(new MailMessage("mysite@nowhere.com", email){Subject="HEllo fool!"});
     }

     public bool ValidateEmail(string email)
     {
         return email.Contains("@");
     }
}

One might wonder why that line of code should be moved. It’s so small. In reality email validation might be larger, but that’s not really important. Following SRP usually makes it easier to follow DRY (Don’t repeat yourself) too, since you make methods smaller and let them solve a specific problem. Hence it’s a lot easier to reuse code.

If we continue to look at the method we’ll see that it also sends an email and is therefore also responsible of delivering the email. Lets move that to a new method too.

public class UserService
{
     public void Register(string email, string password)
     {
          if (!ValidateEmail(email))
              throw new ValidationException("Email is not an email!");

         var user = new User(email, password);
         _database.Save(user);

         SendEmail(new MailMessage("mysite@nowhere.com", email){Subject="HEllo fool!"});
     }

     public virtual bool ValidateEmail(string email)
     {
         return email.Contains("@");
     }

     public bool SendEmail(MailMessage message)
     {
         _smtpClient.Send(message);
     }
}

omg! We did it again. A single line method. Is this sh!t really necessary? Yep, it is. When the application grows you’ll see that you might want some templating. By using a separate method you could replace commonly used macros like #SITEURL#, #PRODUCTNAME# etc in that method instead of duplicating the code in each method that wants to send an email.

Now it’s time to move on and look at the class. Do it follow the SRP? Do it? Nooo, don’t try to fool me. I got some NINJA zkillz. promise! SendEmail and ValidateEmail has nothing to do with a class named UserService. Let’s refactor.


public class UserService
{
     EmailService _emailService;

     public UserService(EmailService emailService)
     {
         _emailService = emailService;
     }
     public void Register(string email, string password)
     {
          if (!_emailService.ValidateEmail(email))
              throw new ValidationException("Email is not an email!");

         var user = new User(email, password);
         _database.Save(user);

         emailService.SendEmail(new MailMessage("mysite@nowhere.com", email){Subject="HEllo fool!"});
     }
}

public class EmailService
{
     public bool virtual ValidateEmail(string email)
     {
         return email.Contains("@");
     }

     public bool SendEmail(MailMessage message)
     {
         _smtpClient.Send(message);
     }
}

Actually I’m unsure of if the ValidateEmail method belongs in the EmailService class. What do you think? Leave a comment.

SRP is nothing that you do once. It’s something that you should keep doing during all types of development (like when writing new code and when you maintain existing one).

Summary

Try to follow the following rules to get a hang of SRP:

  1. Document your code. Using AND’s should make you check your code again.
  2. You should be able to associate all method names with the class/interface name. (A method called ValidateEmail cannot be associated with a class called UserService)
  3. A method should only contain logic that can be associated with the method name.