Thursday, January 22, 2009

I love Paul

1 Cor 6:12

"Everything is permissible for me" — but not everything is beneficial. "Everything is permissible for me" — but I will not be mastered by anything.

I'm an idiot

I worked on a crapload of small ASP.NET projects where I spent a lot of time on user management: add users, edit users, change roles, verify roles and so on. This includes tiny projects like a tiny website where the content of about 10 pages can be edited by the customer (mostly by adding news). They'll never have more than one person doing these edits, their primary business is totally unrelated to computers.

Well, I was reading this article by Scott Hanselman and what do I see:

Does it need an administrative console?, I ask. He says that'd be nice, but it wasn't spec'ed out. He figured this would be pretty low rent. The client even suggested (gasp) that they could just maintain a local XML file.

How on Earth can I be so stupid? I always knew that ASP.NET had this feature... it just never crossed my mind to use it. Dammit.

Yes to mentoring, no to certification

Great article here. I hate the programmers dragging the certification bogey-man every time they have a problem with someone's code. Yeah, we really want to have the problems of the medical field. This article is a breath of fresh air.

Wednesday, January 21, 2009

Take that, ServiceLocator denigrators!

From Inversion of Control Containers and the Dependency Injection pattern by Martin Fowler:

The choice between Service Locator and Dependency Injection is less important than the principle of separating service configuration from the use of services within an application.

Ha! :P

More on MVC

Follow-up to this.

This is the structure I've settled on for the time being:

  • View

  • Controller

  • Service

  • Model

  • Repository

  • Data layer (eg, Linq to SQL)

  • Data storage (eg, SQL database)

The View, Controller and Service communicate through DTOs (aka ViewModels); the View and Controller never see the (domain) Models. I'm not 100% decided about the Service - Repository interface: should the Model objects be the same as the Data Layer objects? For purity reasons I'd say no - it's a bad idea to return Linq to Sql objects to the Service because it will cause problems when trying to switch to (say) Entity Framework. However, I haven't yet worked on a project big enough that the benefits of this separation outweigh the drawback of creating 3 different classes for the same entity (Linq Order, Model Order, DTO Order). I'll have to see what happens when I actually do work on a larger project (with this architecture I mean, I work on huge projects but not with this structure).

The Service has two types of methods:

  • Reads, where it asks the repository for a list of Models (eg Products).

  • Writes, where it asks the repository for a Model and then asks the Model to do something.

This way I avoid the anemic domain anti-pattern - the Service layer can stay thin and let the Models provide (most of) the business logic - querying is not completely a business logic concern. (It is more of a reporting concern instead of a transaction concern.)

Speaking of that, I also need to come up with a way of saying "commit" when I'm doing transactions; each Service method must be a transaction, but I still haven't come up with a way of doing that. (Maybe the EntityService base class - all my Services inherit from it - should have a Commit method that in turns calls for example DataContext.SubmitChanges(). However, that would link the Service to a specific Data Layer, bypassing the repository/ies, which I'd rather not do. Hmm.

I already clarified the structure of the repository (if only to myself, since I haven't posted my last changes to the interfaces and Linq implementation ). In theory, it shouldn't be a big deal (a day or two) to write a Data Layer for, say, SQLite.

Well. Good place to save my current thinking on the subject. Hopefully I'm not too far off-mark.

Wednesday, January 14, 2009

TDD and discipline

In a comment on his article, Karl Seguin has this to say about the usefulness of TDD:

My best suggestion was to unit test as you go, and most of these problems would have ironed themselves out. They never would have gotten the 100+ line controller actions under test, and would have had to address the problem upfront.

Well... unfortunately, that only works for someone who truly believes in the value of TDD, figures out the "hint" - and then is disciplined enough to do the right thing. You know what my reaction is, in these cases? Screw the tests, I have code to write!

Which, of course, is the wrong thing to do. I feel the need for a serenity prayer...

Nice methods in the ServiceLocator class

I just like the way these methods look. I'd take the ServiceLocator class to a job interview :)

  1. /// <summary>  
  2. /// This is used by the application when it needs an instance of T  
  3. /// </summary>  
  4. /// <param name="T">Type that the application needs an instance of</param>  
  5. /// <returns>An instance of T</returns>  
  6. public static object GetInstanceOf(Type T)  
  7. {  
  8.   lock (containerLock)  
  9.     if (container.ContainsKey(T))  
  10.       return container[T]();  
  12.   return CreateInstance(T) ?? (OnTypeNotFound != null ? OnTypeNotFound(T) : null);  
  13. }  
  15. /// <summary>  
  16. /// Creates a new instance of T using the "best" constructor (the one with the biggest number of arguments that are known to the service locator, with the default constructor being the last attempt)  
  17. /// </summary>  
  18. /// <param name="T">Type to instantiate</param>  
  19. /// <returns>A new instance of T or null if no suitable constructor has been found</returns>  
  20. public static object CreateInstance(this Type T)  
  21. {  
  22.   var constructor = GetBestConstructor(T);  
  24.   return constructor != null ? constructor.Invoke() : Activator.CreateInstance(T);  
  25. }  
  27. private static ConstructorInfo GetBestConstructor(Type T)  
  28. {  
  29.   return (from c in T.GetConstructors(BindingFlags.Public)  
  30.           let parameters = c.GetParameters()  
  31.           orderby parameters.Length descending  
  32.           where parameters.All(p => Contains(p.ParameterType))  
  33.           select c)  
  34.     .FirstOrDefault();  
  35. }  
  37. private static object Invoke(this ConstructorInfo constructor)  
  38. {  
  39.   var arguments = constructor  
  40.     .GetParameters()  
  41.     .Select(p => GetInstanceOf(p.ParameterType))  
  42.     .ToArray();  
  44.   return constructor.Invoke(arguments);  
  45. }  

Tuesday, January 13, 2009

Really weird C# code

My ServiceLocator (which has been further updated since my last post on it, so on the off chance that anybody wants the latest version, leave me a comment) can only register functions with no parameters - Func<T>. What happens if one wants to register a function with one or more parameters?

As an example, let's say that I have many possible connection strings, and in some cases when I want an instance of DataContext I have to initialize it with one of those. Since my ServiceLocator doesn't accept functions with parameters, I cannot write

    var dc = ServiceLocator.GetInstanceOf<DataContext>(connectionString); 

Here's a way of transforming a function with parameters to one without: create a function that returns the original function. That is, given

    Func<T1, T> f; 

I need another function that will return f:

    Func<Func<T1, T>> g = () => (a) => f(a);   

So, in order to register my DataContext I am now going to use the following syntax:

    Bind<Func<string, DataContext>>.To(() => (connectionString => new DBDataContext(connectionString))); 

and to retrieve the instance and initialize it correctly, I must use:

    var dc = ServiceLocator.GetInstanceOf<Func<string, DataContext>>()(connectionString); 

Not extremely clear, so some helper functions (and confining in a library of sorts) are probably needed.

Edit: I just made a change to the ServiceLocator code; one can now write:

    var dc = ServiceLocator.GetInstanceOf<string, DataContext>("connectionString");  

This is the overload for one argument:

    public static T GetInstanceOf<T1, T>(T1 arg1) where T : class  
      return GetInstanceOf<Func<T1, T>>()(arg1);  

I only have 3 overloads (up to Func<T1, T2, T3, T>) but that should be enough for anything I might need in the foreseeable future. Oh, and I can still have / use the initial binding too (the one with no connection string). Cool :)

Friday, January 09, 2009

Be careful with sample code

Found this code somewhere, it doesn't matter where:

  1. public override bool ValidateUser(string strName, string strPassword)  
  2. {  
  3.     //This is the custom function you need to write. It can do anything you want.  
  4.     //The code below is just one example.  
  6.     // strName is really an email address in this implementation  
  8.     bool boolReturn = false;  
  10.     // Here is your custom user object connecting to your custom membership database  
  11.     MyUserProvider oUserProvider = new MyUserProvider();  
  12.     MyUser oUser = oUserProvider.FindUser(strName);  
  14.     if (oUser == null)  
  15.         return boolReturn;  
  17.     // Here is your custom validation of the user and password  
  18.     boolReturn = oUser.ValidateUsersPasswordForLogon(strPassword);  
  20.     return boolReturn;  
  21. }  

The comments are there because it's a sample, not because they should be there in production code, so that's fine. My problem is with the boolReturn variable.

  1. bool boolReturn = false;  
  3. // Here is your custom user object connecting to your custom membership database  
  4. MyUserProvider oUserProvider = new MyUserProvider();  
  5. MyUser oUser = oUserProvider.FindUser(strName);  
  7. if (oUser == null)  
  8.     return boolReturn;  

First of all, one rule (Code Complete should really be required reading for programmers) is that you want to keep assignments to a variable as close as possible to the place where the variable is used. The oUserProvider and oUser variables have nothing to do with the boolReturn, so let's switch the order:

  1. // Here is your custom user object connecting to your custom membership database  
  2. MyUserProvider oUserProvider = new MyUserProvider();  
  3. MyUser oUser = oUserProvider.FindUser(strName);  
  5. bool boolReturn = false;  
  6. if (oUser == null)  
  7.     return boolReturn;  

Ok... is the boolReturn variable used anywhere after the return? Not before it's reassigned. That means that, for all intents and purposes, the scope of the variable boolReturn ends here, and a *new* boolReturn variable is created. (Think about it.)

Let's rewrite the code accordingly (think it through to verify that nothing was changed in the meaning of the code):

  1. public override bool ValidateUser(string strName, string strPassword)  
  2. {  
  3.     //This is the custom function you need to write. It can do anything you want.  
  4.     //The code below is just one example.  
  6.     // strName is really an email address in this implementation  
  8.     // Here is your custom user object connecting to your custom membership database  
  9.     MyUserProvider oUserProvider = new MyUserProvider();  
  10.     MyUser oUser = oUserProvider.FindUser(strName);  
  12.     bool boolReturn = false;  
  13.     if (oUser == null)  
  14.         return boolReturn;  
  16.     // Here is your custom validation of the user and password  
  17.     boolReturn2 = oUser.ValidateUsersPasswordForLogon(strPassword);  
  19.     return boolReturn2;  
  20. }  

Progress. This change makes visible another problem: why set a variable if you're only using it once? (Ok, it makes sense for complicated calculations, but it's not the case here.)

  1. public override bool ValidateUser(string strName, string strPassword)  
  2. {  
  3.     //This is the custom function you need to write. It can do anything you want.  
  4.     //The code below is just one example.  
  6.     // strName is really an email address in this implementation  
  8.     // Here is your custom user object connecting to your custom membership database  
  9.     MyUserProvider oUserProvider = new MyUserProvider();  
  10.     MyUser oUser = oUserProvider.FindUser(strName);  
  12.     if (oUser == null)  
  13.         return false;  
  15.     // Here is your custom validation of the user and password  
  16.     return oUser.ValidateUsersPasswordForLogon(strPassword);  
  17. }  

Frankly, I find the ?: operator to be a great help, so my code would tend to look like the next example, but I understand some people get confused by it, so take your pick:

  1. public override bool ValidateUser(string strName, string strPassword)  
  2. {  
  3.     //This is the custom function you need to write. It can do anything you want.  
  4.     //The code below is just one example.  
  6.     // strName is really an email address in this implementation  
  8.     // Here is your custom user object connecting to your custom membership database  
  9.     MyUserProvider oUserProvider = new MyUserProvider();  
  10.     MyUser oUser = oUserProvider.FindUser(strName);  
  12.     return oUser == null  
  13.         ? false  
  14.         : oUser.ValidateUsersPasswordForLogon(strPassword);  
  15. }  

Finally, what's up with the (bad) Hungarian notation? The fact that the name (or email, whatever) is a string or something else is relevant for the compiler, not for the programmer. The good Hungarian notation can be useful, but most people seem to only know about the bad variant. Let's get rid of it altogether:

  1. public override bool ValidateUser(string name, string password)  
  2. {  
  3.     var userProvider = new MyUserProvider();  
  4.     var user = userProvider.FindUser(name);  
  6.     return user == null  
  7.         ? false  
  8.         : user.ValidateUsersPasswordForLogon(password);  
  9. }  

To me, this looks much better than the original. Doesn't it?

Thursday, January 08, 2009

MVC explained

... to myself, as I'm trying to learn it :)

This is how I'm organizing a project I'm currently working on:

  • View - this is used to render the model as HTML, since this is an ASP.NET project. The View can contain view logic, like displaying negative numbers in red.

  • ViewModel - this is not necessarily the domain model (database table), but a view-specific model. An Order view, for example, needs to display data from an Order, multiple OrderDetails, a Customer and possibly a Seller (and maybe more). Right now, I handle this case by making a composite class with several properties:

  1. public class OrderModel  
  2. {  
  3.   public Order Order { getset; }  
  4.   public IEnumerable<OrderDetail> Details { getset; }  
  5.   public Customer Customer { getset; }  
  6.   public Address ShippingAddress { getset; }  
  7.   public bool CanEditShipping { getset; }  
  8. }  

This is basically a DataTransferObject and is used as such by the view. I realize it would be safer if I created an immutable struct with the necessary data and pass that to the view, but I am too lazy and don't have a tool to do that for me right now. I think this is what Microsoft tried to do with their ViewData dictionary, since it's (normally) strictly write-only for the controller and read-only for the view. This might be easier in C# 4.0, because I can discover properties at runtime, so I can write something like this:

  1. return View(new { Order = order, OrderDetails = order.Details, Customer = customer, ShippingAddress = order.ShippingAddress, CanEditShipping = true });  

and still use Model.CanEditShipping in the view. (It still requires some discipline to use Model.OrderDetails instead of Model.Order.Details, of course.)

  • Controller - this has three responsibilities in my application: to use the business logic to retrieve the domain models (representing records), combine those into the model as defined above and choose the right view to apply or the correct path to re-route to (this is the "controller logic"). The controller is very "skinny", usually a couple of lines per action.

  • Repository - this is the business logic.

  • Domain model - aka tables / records / plain C# objects. Can contain "model logic", like a GetMainImage() method on the Product object that can decide to "promote" an image as main if none of those associated with the product have been flagged as such.

Any glaring mistakes?

Edit: I just found this article by Jimmy Bogard

Edit (Jan 16, 2009): Oopsie. It seems that I'm going to have to revise this.

Wednesday, January 07, 2009

ServiceLocator anti-pattern?

I was trying to reply to this article but for some reason I cannot leave a comment there... there's no error message, my comment is simply ignored. So I'm replying here, hopefully I'll be able at some point to at least reply with the address of this article.

Ok... so if I understand this correctly, Nick's problems with the ServiceLocator pattern are:

1) Because it's global, it cannot be tested
2) Sometimes we want to return the same instance for all Resolve() calls, sometimes a thread- or HttpContext-specific instance, sometimes a new instance each time; can't do that with a dictionary

(I'm ignoring the points in the "Problems with..." section because I can't understand any of them. "All of the robustness that is achieved by forcing components to publicly declare dependencies is lost." - why? "When dependencies are retrieved from a global container, the container does not know the identity of the requestor, and is forced to return the same implementation every time." - this is a GOOD thing. You want an instance of X, it shouldn't depend on the requestor - if it does, make many kinds of Xs - it's obviously not the same X. "Concurrency, re-entrancy, circular reference detection, and component lifetime management are much harder/messier in a global container based solution." - again, why?)

It's weird, a real coincidence, but a few days after he wrote this article (which I've only discovered half an hour ago) I wrote a ServiceLocator implementation. This was my first post on the subject, followed by this one and finally this. As shown in the last post, this is one way it can be used:

  1. Bind<IProductRepository>  
  2.   .To(() => new ProductRepository())  
  3.   .CacheBy(InstanceScope.HttpContext);  

Can you take a look at this and point out where I'm wrong? I agree, right now I do NOT use any kinds of magic to get rid of the

  1. var repository = ServiceLocator.GetInstanceOf<IProductRepository>();  

calls, but I do not actually find that a problem (at least not yet). In fact, if I could find a way that doesn't look forced to replace it with something else, I will probably look into it. (Decorating properties or constructors with [Inject] attributes looks a little weird, but maybe I'm mistaken.)

I've started using this in a project and, so far, everything seems to be OK. I'm definitely open to the idea that problems are lurking in the background though, so I'll appreciate any hint you can give me.

Returning a default value in case of errors

Ever wrote something like this?

  1. if (a != null && a.b != null && a.b.c != null)  
  2.   result = a.b.c.d;  
  3. else  
  4.   result = null;  

I hate it. Given that I'm processing some XML files whose schemas I cannot change, there's not much to be done but bite the bullet.

This page gave me the idea for the following helper functions:

  1. public static T Eval<T, ExceptionClass>(Func<T> func, Func<ExceptionClass, T> onError)  
  2.   where ExceptionClass: Exception  
  3. {  
  4.   try  
  5.   {  
  6.     return func();  
  7.   }  
  8.   catch (ExceptionClass e)  
  9.   {  
  10.     return onError(e);  
  11.   }  
  12. }  
  14. public static T Eval<T>(Func<T> func, Func<Exception, T> onError)  
  15. {  
  16.   return Eval<T, Exception>(func, onError);  
  17. }  
  19. public static T Eval<T>(Func<T> func, T defaultValue)  
  20. {  
  21.   return Eval<T, Exception>(func, e => defaultValue);  
  22. }  

Now I can write this:

  1. result = Helper.Eval(() => a.b.c.d, null);  

As an aside, it's really annoying not being able to write something like this:

  1. public static T Something<T>(Func<T[]> func)  

instead of writing say 5 overloads for Func<T> to Func<T1, T2, T3, T4, T>.

Saturday, January 03, 2009

More on ServiceLocator - caching

I got an idea while watching this webcast with Rob Conery and Jeremy Miller - Jeremy was talking about StructureMap and how you can set caching with it. It took me a few hours but I managed to duplicate that functionality. Of course, it doesn't work when you call Discover() - that one will still be the regular un-cached call (I think Jeremy calls it PerInstance).

This is an usage example:

  1. Bind<DataContext> 
  2.   .To(() => new DBDataContext()) 
  3.   .CacheBy(InstanceScope.Singleton); 
  4. Bind<IProductRepository> 
  5.   .To(() => new ProductRepository()) 
  6.   .CacheBy(InstanceScope.HttpContext); 

Here are the helper classes in all their glory:

  1. using System; 
  2. using System.Collections.Generic; 
  3. using System.Threading; 
  4. using System.Web; 
  6. namespace Helper 
  7.   // usage: 
  8.   // Bind<ISomething> 
  9.   //   .To(() => new Something()) 
  10.   //   .CacheBy(InstanceScope.ThreadLocal); 
  12.   /// <summary> 
  13.   /// Fluent interface simplifying the registration of implementations 
  14.   /// </summary> 
  15.   /// <typeparam name="T">Type to implement</typeparam> 
  16.   /// <example>Bind&lt;IRepository&gt;.To(() => new Repository());</example> 
  17.   public static class Bind<T> where T : class 
  18.   { 
  19.     /// <summary> 
  20.     /// Maps T to Func&lt;T&gt; 
  21.     /// </summary> 
  22.     /// <param name="func">Function returning an instance of T</param> 
  23.     public static BindHelper<T> To(Func<T> func) 
  24.     { 
  25.       ServiceLocator.Register(func); 
  27.       return new BindHelper<T>(func); 
  28.     } 
  29.   } 
  31.   public class BindHelper<T> where T : class 
  32.   { 
  33.     private readonly Func<T> func; 
  35.     public BindHelper(Func<T> func) 
  36.     { 
  37.       this.func = func; 
  38.     } 
  40.     public void CacheBy(InstanceScope scope) 
  41.     { 
  42.       ServiceLocator.Unregister<T>(); 
  43.       ServiceLocator.Register(Memoize(scope)); 
  44.     } 
  46.     private static readonly Dictionary<string, object> cache = new Dictionary<string, object>(); 
  48.     private Func<T> Memoize(InstanceScope scope) 
  49.     { 
  50.       Func<Type, string> GetTypeKey; 
  52.       switch (scope) 
  53.       { 
  54.         case InstanceScope.NoCaching: 
  55.           return func; 
  57.         case InstanceScope.Singleton: 
  58.           GetTypeKey = type => type.ToString(); 
  59.           break
  61.         case InstanceScope.ThreadLocal: 
  62.           GetTypeKey = type => type + "_" + Thread.CurrentThread.ManagedThreadId; 
  63.           break
  65.         case InstanceScope.HttpContext: 
  66.           GetTypeKey = type => type + "_" + HttpContext.Current; 
  67.           break
  69.         case InstanceScope.Hybrid: 
  70.           GetTypeKey = type => type + "_" + HttpContext.Current ?? Thread.CurrentThread.ManagedThreadId.ToString(); 
  71.           break
  73.         default
  74.           GetTypeKey = type => type.ToString(); 
  75.           break
  76.       } 
  78.       return () => 
  79.                { 
  80.                  object result; 
  81.                  if (!cache.TryGetValue(GetTypeKey(typeof(T)), out result)) 
  82.                    cache[GetTypeKey(typeof(T))] = func(); 
  84.                  return cache[GetTypeKey(typeof(T))] as T; 
  85.                }; 
  86.     } 
  87.   } 

By all means, if you can figure out a way to simplify this, let me know :)

Ah, I forgot the InstanceScope enum:

  1. namespace Helper 
  2.   public enum InstanceScope 
  3.   { 
  4.     HttpContext, 
  5.     Hybrid, 
  6.     ThreadLocal, 
  7.     Singleton, 
  8.     NoCaching 
  9.   }