вторник, 24 сентября 2013 г.

IQueryable is root of many problems

It's often suggested to avoid unnecessary abstractions in your projects. Some respectful community members like to repeat it over and over again. But how to distinguish between required and unnecessary abstraction? What values can abstraction bring in?

Let's take a look at sample code which uses NHibernate's session to access data.

 internal sealed class TaskCoordinator
 {
  private readonly ISession _session;
 
  public TaskCoordinator(ISession session)
  {
   _session = session;
  }
 
  public void DoSomething()
  {
   /* ... */
 
   var tasks = GetRunningTasks(1);
 
   /* ... */
 
   foreach (var task in tasks)
   {
    var previousTask = GetPreviousTask(task);
 
    /* ... */
   }
  }
 
  private Task[] GetRunningTasks(int workflowId)
  {
   var utcNow = DateTime.UtcNow;
   return _session.Query<Task>()
    .Where(t => t.IsActive)
    .Where(t => t.WorkflowId == workflowId)
    .Where(t => t.StartsAt <= utcNow)
    .Where(t => t.ExpiresAt > utcNow)
    .ToArray();
  }
 
  private Task GetPreviousTask(Task task)
  {
   return _session.Query<Task>()
    .Where(t => t.WorkflowId == task.WorkflowId)
    .Where(t => t.StartsAt < task.StartsAt)
    .Where(t => t.ExpiresAt <= task.StartsAt)
    .FirstOrDefault();
  }
 }


Problem is: should we use session directly or should be hide it under another level of abstraction.
But before making any decision let me ask you a few questions:

  1. Except type safety, what is the difference between these LINQ-queries and SQL statements used directly in code?
  2. Without studying whole codebase, can you tell me what are use cases and what indices should be created in database to cover these use cases?
  3. Can you estimate L2 cache hit ratio for your queries?
  4. Can you easily (easily means isolated, without building two-page test setup) test buggy method GetPreviousTask?
  5. Can you write unit (means without involving database) tests for TaskCoordinator?
These questions and your answers help you make informed decision and choose between previous implementation and introducing another level of abstraction:


 public interface ITaskRepository
 {
  Task[] GetRunningTasks(int workflowId);
  
  Task GetPreviousTask(Task task);
 }
 
 internal sealed class TaskCoordinator
 {
  private readonly ITaskRepository _repository;
 
  public TaskCoordinator(ITaskRepository repository)
  {
   _repository = repository;
  }
 
  public void DoSomething()
  {
   /* ... */
 
   var tasks = _repository.GetRunningTasks(1);
 
   /* ... */
 
   foreach (var task in tasks)
   {
    var previousTask = _repository.GetPreviousTask(task);
 
    /* ... */
   }
  }
 }

Actually title is provoking, it's not the IQueryable problem, it's a problem of any wide unbounded contract.
Wider Two Column Modification courtesy of The Blogger Guide