Oren Eini

CEO of RavenDB

a NoSQL Open Source Document Database

Get in touch with me:

oren@ravendb.net +972 52-548-6969

Posts: 7,520
|
Comments: 51,141
Privacy Policy · Terms
filter by tags archive
time to read 2 min | 289 words

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.

You might have wondered why I named this blog series the way I did, I named it because of the method outline below. Please note that I had to invent a new system to visualize the data access behavior on this system:

image

  • In red, we have queries that are executed once: 3 queries total.
  • In aqua, we have queries that are executed once for each item in the order: 2 queries per each product in the order.
  • In purple, we have queries that are executed once for each attribute in each of the products in the order: 1 query per attribute per product in the order.

Now, just to give you some idea, let us say that I order 5 items, and each item have 5 attributes…

We get the following queries:

  • 3 queries – basic method cost
  • 10 queries – 2 queries per each product
  • 25 queries – 1 query for each attribute for each product

Totaling in 38 queries for creating a fairly simple order. After seeing this during the course review of the application, I have used that term for this method, because it is almost too easy to make those sort of mistakes.

time to read 2 min | 321 words

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.

As you might have noticed, I am pretty big fan of the NHibernate Futures feature (might be because I wrote it Smile), so I was encouraged when I saw it used in the project. It can drastically help the performance of the system.

Except, that then I saw how it is being used.

image

Which is then called from CurrencyService:

image

Which is then called…

image

Do you see that? We take the nice future query that we have, and turn it into a standard query, blithely crashing any chance to actually optimize the data access of the system.

The problem is that from the service API, there is literally nothing to tell you that you need to order your calls so all the data access happens up front, and it is easy to make mistakes such as this. The problem is that there is meaning lost at any additional abstraction layer, and pretty soon whatever good intention you had when you wrote a particular layer, 7 layers removed, you can’t really remember what is the best way to approach something.

time to read 6 min | 1193 words

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:

image

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.

time to read 5 min | 868 words

Originally posted at 3/10/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. The problem is that this system seems to be drastically more complicated than it should be.

In this case, I want to look at the type of API that is exposed:

image

If this reminds you of the bad old days of having only Stored Procedure API available, that is not by chance. Far worst than that, however, is the call paths where this is used.

  • IEmailTemplateRepository.Get(EmailTemplateLookup) is implemented as
    public EmailTemplate Get(EmailTemplateLookup emailId)
    {
        return Get((int)emailId);
    }
    and is only used in:
    • EmailService.Get(EmailTemplateLookup) whose implementation is:
      public EmailTemplate GetEmail(EmailTemplateLookup template)
      {
          return emailTemplateRepository.Get(template);
      }
  • ICategoryRepository.GetParentCategories is only used from:
    • CategoryService.GetParentCategories which is implemented as:
      public IEnumerable<Category> GetParentCategories()
      {
          IEnumerable<Category> categories = categoryRepository.GetParentCategories();
      
          return categories;
      }
  • ICurrencyRepository.GetEnabledCurrencies is only used from:
    • CurrencyService.GetEnabledCurrencies which is implemented as:
      public IEnumerable<Currency> GetEnabledCurrencies()
      {
           return currencyRepository.GetEnabledCurrencies();
      }
  • For that matter, let us take a look at the entire CurrencyService class, shall we?

    public class CategoryService : ICategoryService
    {
        private readonly ICategoryRepository categoryRepository;
    
        public CategoryService(ICategoryRepository categoryRepository)
        {
            this.categoryRepository = categoryRepository;
        }
    
        public IList<Category> GetCategories()
        {
            return categoryRepository.GetAll();
        }
    
        public Category GetCategory(int id)
        {
            return categoryRepository.Get(id);
        }
    
        public void SaveOrUpdate(Category categoryModel)
        {
            categoryRepository.SaveOrUpdate(categoryModel);
        }
    
        public void Delete(Category category)
        {
            categoryRepository.Delete(category);
        }
    
        public IEnumerable<Category> GetParentCategories()
        {
            IEnumerable<Category> categories = categoryRepository.GetParentCategories();
    
            return categories;
        }
    }

    To be honest, I really don’t see the point.

    Now, just a hint on the next few posts, there are places where I think wrapping the usage of the NHibernate API was a good idea, even if I strongly disagree with how this was done.

    time to read 2 min | 291 words

    Originally posted at 3/10/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. The problem is that this system seems to be drastically more complicated than it should be.

    I am going to focus  on different parts of the system in each of those posts. In this case, I want to focus on the very basis for the application data access:

    image

    Are you kidding me? This is before you sit down to write a single line of code, mind you. Just the abstract definitions for everything makes my head hurt.

    It really hits you over the head when you get this trivial implementation:

    public class EmailTemplateRepository : Repository<EmailTemplate>, IEmailTemplateRepository
    {
        public EmailTemplate Get(EmailTemplateLookup emailId)
        {
            return Get((int)emailId);
        }
    }

    Yes, this is the entire class.  I am sorry, but I really don’t see the point. The mental weight of all of this is literally crashing.

    FUTURE POSTS

    No future posts left, oh my!

    RECENT SERIES

    1. Challenge (75):
      01 Jul 2024 - Efficient snapshotable state
    2. Recording (14):
      19 Jun 2024 - Building a Database Engine in C# & .NET
    3. re (33):
      28 May 2024 - Secure Drop protocol
    4. Meta Blog (2):
      23 Jan 2024 - I'm a JS Developer now
    5. Production Postmortem (51):
      12 Dec 2023 - The Spawn of Denial of Service
    View all series

    RECENT COMMENTS

    Syndication

    Main feed Feed Stats
    Comments feed   Comments Feed Stats
    }