Originally posted at 3/11/2011
This time, this is a review of the Sharp Commerce application. Again, I have stumbled upon the application by pure chance, and I have very little notion about who wrote it.
In this case, I want to focus on the ProductRepository:
In particular, those methods also participate in Useless Abstraction For The Sake Of Abstraction Anti Pattern. Here is how they are implemented:
public AttributeItem GetAttributeItem(int attributeItemId)
{
return Session.Get<AttributeItem>(attributeItemId);
}
public Attribute GetAttribute(int attrbuteId)
{
return Session.Get<Attribute>(attrbuteId);
}
public IEnumerable<Attribute> GetAllAttributes()
{
return Session.QueryOver<Attribute>()
.Future<Attribute>();
}
public void SaveOrUpdate(Attribute attribute)
{
Session.SaveOrUpdate(attribute);
}
And here is how they are called (from ProductService):
public AttributeItem GetAttributeItem(int attributeItemId)
{
return productRepository.GetAttributeItem(attributeItemId);
}
public Attribute GetAttribute(int attrbuteId)
{
return productRepository.GetAttribute(attrbuteId);
}
public void SaveAttribute(Attribute attribute)
{
productRepository.SaveOrUpdate(attribute);
}
public IList<Product> GetProducts()
{
return productRepository.GetAll();
}
public Product GetProduct(int id)
{
return productRepository.Get(id);
}
public void SaveOrUpdate(Product product)
{
productRepository.SaveOrUpdate(product);
}
public void Delete(Product product)
{
productRepository.Delete(product);
}
public IEnumerable<Attribute> GetAllAttributes()
{
return productRepository.GetAllAttributes();
}
Um… why exactly?
But as I mentioned, this post is also about the proper usage of abstracting the OR/M. A repository was originally conceived as a to abstract away messy data access code into nicer to use code. The product repository have one method that actually do something meaningful, the Search method:
public IEnumerable<Product> Search(IProductSearchCriteria searchParameters, out int count)
{
string query = string.Empty;
if (searchParameters.CategoryId.HasValue && searchParameters.CategoryId.Value > 0)
{
var categoryIds = (from c in Session.Query<Category>()
from a in c.Descendants
where c.Id == searchParameters.CategoryId
select a.Id).ToList();
query = "Categories.Id :" + searchParameters.CategoryId;
foreach (int categoryId in categoryIds)
{
query += " OR Categories.Id :" + categoryId;
}
}
if (!string.IsNullOrEmpty(searchParameters.Keywords))
{
if (query.Length > 0)
query += " AND ";
query += string.Format("Name :{0} OR Description :{0}", searchParameters.Keywords);
}
if (query.Length > 0)
{
query += string.Format(" AND IsLive :{0} AND IsDeleted :{1}", true, false);
var countQuery = global::NHibernate.Search.Search.CreateFullTextSession(Session)
.CreateFullTextQuery<Product>(query);
var fullTextQuery = global::NHibernate.Search.Search.CreateFullTextSession(Session)
.CreateFullTextQuery<Product>(query)
.SetFetchSize(searchParameters.MaxResults)
.SetFirstResult(searchParameters.PageIndex * searchParameters.MaxResults);
count = countQuery.ResultSize;
return fullTextQuery.List<Product>();
}
else
{
var results = Session.CreateCriteria<Product>()
.Add(Restrictions.Eq("IsLive", true))
.Add(Restrictions.Eq("IsDeleted", false))
.SetFetchSize(searchParameters.MaxResults)
.SetFirstResult(searchParameters.PageIndex * searchParameters.MaxResults)
.Future<Product>();
count = Session.CreateCriteria<Product>()
.Add(Restrictions.Eq("IsLive", true))
.Add(Restrictions.Eq("IsDeleted", false))
.SetProjection(Projections.Count(Projections.Id()))
.FutureValue<int>().Value;
return results;
}
}
I would quibble about whatever this is the best way to actually implement this method, but there is little doubt that something like this is messy. I would want to put this in a very distant corner of my code base, but it does provides a useful abstraction. I wouldn’t put it in a repository, though. I would probably put it in a Search Service instead, but that isn’t that important.
What is important is to understand where there is actually a big distinction between code that merely wrap code for the sake of increasing the abstraction level and code that provide some useful abstraction over an operation.