Over a million developers have joined DZone.
{{announcement.body}}
{{announcement.title}}

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

DZone's Guide to

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

·
Free Resource

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 Oren Eini, DZone MVB. See the original article here.

Opinions expressed by DZone contributors are their own.

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

{{ parent.tldr }}

{{ parent.urlSource.name }}