How much of a performane hit will i take from casting when trying to make this code mistake proof?

Question

I have the following code:

public class CrudModel<T> : ICrudModel<T> where T : DomainBase
{
    public IQueryable<T> GetAll()
    {
        return Repository.Query<T>();
    }
}

the issue is that some of the objects (T) I need to do an extra filter so I created a separate method like this:

public IEnumerable<TR> GetAllwithinOrg<TR>() where TR : DomainBase, IFilterable
{
    var items = Repository.Query<TR>();
    return FilterwithinOrg(items);
}

where filter method looks like this:

public IEnumerable<TResult> FilterwithinOrg<TResult>(IEnumerable<TResult> linked) where TResult :  IFilterable
{
    var dict = GetDict();
    return linked.Where(r => dict.ContainsKey(r.Id));
}

this all works fine but the issue is that I have to remember to call method 1 or method 2 (based on if the object supports the IFilterable interface

On this question, I got the suggestion to do this:

public IQueryable<T> GetAll()
{
    var items = Repository.Query<T>();
    if (typeof(IFilterable).IsAssignableFrom(typeof(T)))
    {
        items = FilterwithinOrg(items.Cast<IFilterable>()).Cast<T>().AsQueryable();
    }
    return items;
}

so I can support both use cases in one method. This seems to work but I am trying to understand what type of performance hit that I am going to take by doing this

items.Cast<IFilterable>()).Cast<T>().AsQueryable()

If it's bad then I will deal with remembering to call 2 separate methods from the outside but obvious it would be convenient to just have one. Any suggestions?

I think I will leave it in just as a backup if I forget to call the second method but wanted to again see if I can keep it to just one if possible to make it simpler for the outside caller.

Use profiler to determine performance hits. It gives you right information. — Kirill Bestemyanov, Apr 28 '13 at 06:59

Sunny · Accepted Answer · 2013-04-28T12:16:28.017

1

How about having another method with where clause in the CrudModel class.

public IEnumerable<T> GetAll<T>(Func<T, bool> whereClause) where T : DomainBase
{
    var items = Repository.Query<T>();
    return items.Where(whereClause);
}

And call using

List<int> intList = new List<int>() { 1 };
intList.GetAll<int>((i) => sampledict.ContainsKey(i));

I felt it is not proper to make things complex by having logic cramped into one single GetAll method and since CrudModel seems to be generic, better to have generic method that accepts any condition.

edited Apr 28 '13 at 12:16

answered Apr 28 '13 at 05:29

Sunny

4,765
5
37
72

I appreciate your point but if i am going to do this, the caller needs to know about how to filter by org here. I don't wan't the caller to have to know about this implementation and access the Dict() cache as i want that centralized (it gets called lots of time) – leora Apr 28 '13 at 11:38
In that case, I would create a extension method that uses dictionary and calls GetAll. If you got other filter in future do you again modify getall method. – Sunny Apr 28 '13 at 12:16
Again, I understand your point but this is a specific filter. I am sharing the database across multiple teams and this filter gives results just for that specific team. – leora Apr 28 '13 at 16:17

score 0 · Answer 2 · answered Apr 28 '13 at 07:59

0

First, I think it is a bit strange that you have a function GetAll, but for certain types, you start filtering, resulting in not getting all :)

Besides that, I do not think you have big performance loss... In essense you do one extra check inside you GetAll-method: typeof(IFilterable).IsAssignableFrom(typeof(T)) is like an ordinary cast. You will hardly feel it.

Perhapse the filter itself could be improved. You create a dictionary. Does the dictionary have the same values every call, or does it change. And why a dictionary if you only use the keys, and not the values? What about a HashSet<T>?

answered Apr 28 '13 at 07:59

Martin Mulder

12,642
3
25
54

the GetDict() is getting values from a cache so not worried there. Is Hashset really that much faster. Most of the stuff i have read says is pretty much the same – leora Apr 28 '13 at 11:32
@Leora. Correct: both are equally fast. But a Dictionary (keys and values) consumes more memory than a HashSet (only keys). – Martin Mulder Apr 28 '13 at 11:34

score 0 · Answer 3 · answered Apr 28 '13 at 12:28

0

Casting time can be neglected comparing to the time spent for database query. However, you are querying the whole table and filter in-memory according to your code here:

public IEnumerable<TResult> FilterwithinOrg<TResult>(IEnumerable<TResult> linked) where TResult :  IFilterable
{
    var dict = GetDict();
    return linked.Where(r => dict.ContainsKey(r.Id));
}

Remember that you need to filter query, not list, so you should change the method to accept and return IQueryable<TResult> instead like:

public IQueryable<TResult> FilterwithinOrg<TResult>(IQueryable<TResult> linked) where TResult :  IFilterable
{
    var dictKeys = GetDict().Keys.ToList();
    return linked.Where(r => dictKeys.Contains(r.Id));
}

Noted that the filter expression has to have equivalent SQL expression or runtime-error will occur.

answered Apr 28 '13 at 12:28

tia

9,518
1
30
44

Yeah . . I need to do the filter in code as the filter in SQL winds up being slower after doing a number of tests . . The IEnumerable is on purpose. . . – leora Apr 28 '13 at 16:16
@leora I doubt the method of your test. The point is that loading full table from the database is rarely a good idea except that you know the table size will always small or for caching. Imagine if you have 10000+ products in a table and you have to load it all from database just for only 10 products you want, and you have to do this for every query. It doesn't make sense at all. – tia Apr 29 '13 at 03:28
I am using a second level cache so wind up loading the whole thing into memory anyway – leora Apr 30 '13 at 02:12

How much of a performane hit will i take from casting when trying to make this code mistake proof?

3 Answers3