Over a million developers have joined DZone.

The wages of sin: Proper and improper usage of abstracting an OR/M

·

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.

Topics:

Published at DZone with permission of Ayende Rahien, DZone MVB. See the original article here.

Opinions expressed by DZone contributors are their own.

The best of DZone straight to your inbox.

SEE AN EXAMPLE
Please provide a valid email address.

Thanks for subscribing!

Awesome! Check your inbox to verify your email so you can start receiving the latest in tech news and resources.
Subscribe

{{ parent.title || parent.header.title}}

{{ parent.tldr }}

{{ parent.urlSource.name }}