views:

61

answers:

1

This is confusing me, so this question will probably be confusing.

I have a an application that uses implementations of an IJob interface to accomplish different tasks.

public interface IJob
{
  int Id { get; set; }
  string Name { get; set; }
  void Run();
}

I am using the Castle.Windsor.WindsorContainer to resolve these implementations, and using the service id to help identify them.

WindsorContainer container = new WindsorContainer(new XmlInterpreter());
IJob jobToExecute = container.Resolve<IJob>("nameOfJob");

I wrote a little generic extension method that simply puts the values of SQL columns into their corresponding properties.

    public static void MapTo<T>(this DbDataReader reader, ref T instance) where T : class
    {
        Type objectType = typeof(T);

        foreach (PropertyInfo propertyInfo in objectType.GetProperties())
        {
            if (propertyInfo.CanWrite)
            {
                int ordinal = -1;
                try
                {
                    ordinal = reader.GetOrdinal(propertyInfo.Name);
                    object value = reader[ordinal] == DBNull.Value ? null : reader[ordinal];
                    propertyInfo.SetValue(instance, value, null);
                }
                catch (IndexOutOfRangeException ex)
                {
                    continue;
                }

            }
        }
    }

Now, because you can't instantiate an instance of an interface, passing an IJob to this method won't work. However, in order to gain the benefits of the IoC container, I need to do everything in my repository using the IJob interface. So, I wrote with this to resolve the IJob implementation, and pass it to the MapTo method to populate the necessary properties:

    public IJob GetJobById(int id)
    {
        string cmdTxt = "SELECT Id, Name, Description, DateStarted, ScheduledCompletion, Completed FROM Jobs WHERE Id = @id";

        using (DbCommand cmd = _dataFactory.CreateCommand(cmdTxt))
        {
            _dataFactory.AddParam(cmd, "id", id, DbType.Int32);
            using (DbDataReader rdr = cmd.ExecuteReader())
            {
                if (rdr.Read())
                {
                    IJob job = _container.Resolve<IJob>("job.implementation");
                    rdr.MapTo<IJob>(ref job);
                    return job;
                }
                else
                {
                    return null;
                }
            }
        }
    }

Is this an OK design decision? Do you see any problems?

+1  A: 

Well, for one, calling methods via reflection is usually not nice... and it looks like you're using Windsor as a type dictionary, which it is not...

I would write a non-generic MapTo (which would take a Type as parameter) that operates on an already-existing instance (when you create a new instance with Activator.CreateInstance you discard the instance Windsor had resolved) and then use it from the ComponentCreatedEvent event in IKernel. Something like this:

container.Kernel.ComponentCreated += (model, instance) => {
  if (model.Service == typeof(IJob)) {
    // select id,name from jobs where id = model.Name
    // use MapTo to fill id,name into instance
  }
}
Mauricio Scheffer
I noticed I was wasting the instance that Windsor resolved (which is one of the reason I posted here). The problem I was having was this: If I create a non-generic MapTo method, I guess I'll have to create one for each domain object (one that maps Job, one that maps Result, etc) and just put them in the respective repositories? I edited the MapTo and GetJobById methods in a way that doesn't discard the Windsor resolved instance. What are your thoughts?
scottm
You don't need to write multiple MapTo methods. You can get the type from instance.GetType() instead of typeof(T). The rest is the same.
Mauricio Scheffer
yeah, I think I got it figured out so I don't have to.
scottm