Ayende @ Rahien

My name is Oren Eini
Founder of Hibernating Rhinos LTD and RavenDB.
You can reach me by phone or email:


+972 52-548-6969

, @ Q c

Posts: 6,128 | Comments: 45,550

filter by tags archive

Elegant Code

time to read 1 min | 165 words

I just finished writing this, and I find it very pleasing to look at:

public class EntitiesToRepositories
	public static void Register(
		IWindsorContainer windsorContainer,
		ISessionFactory sessionFactory,
		Type repository,
		params Assembly[] assemblies
			throw new ArgumentException("Repository must be a type inheriting from IRepository<T>, " + 
"and must be an open generic type. Sample: typeof(NHRepository<>).
"); foreach (IClassMetadata meta in sessionFactory.GetAllClassMetadata().Values) { Type mappedClass = meta.GetMappedClass(EntityMode.Poco); if (mappedClass == null) continue; foreach (Type interfaceType in mappedClass.GetInterfaces()) { if(IsDefinedInAssemblies(interfaceType, assemblies)==false) continue; windsorContainer.Register( Component.For(typeof(IRepository<>).MakeGenericType(interfaceType)) .ImplementedBy(repository.MakeGenericType(interfaceType)) .CustomDependencies(Property.ForKey("ConcreteType").Eq(mappedClass)) ); } } } private static bool IsDefinedInAssemblies(Type type, Assembly[] assemblies) { return Array.IndexOf(assemblies, type.Assembly) != -1; } }


Bogdan Pietroiu

Hi Ayende,

When you coded


you meant


imho, I am afraid that this approach does not play well with separation of concerns alowing a mix of data model layer and business layer


First thoughts when seeing your codesnippet, either:

a) The indent-formatting is messed up in the post (way to deep)


b) You work at a wide, wide, widescreen ;-)

Personally I prefer 4 chars deep indents

Grimace of Despair

Maybe a stupid question, but do you consciously use "== false" instead of negation for readability? Of is it just what popped to mind?

Francois Tanguay

And I imagine your first if was negated. Otherwsie you'll throw the exception quite often.

Fabian Schmied

Have you actually managed to get that first block of code to throw? Francois seems to be right - to fit the exception message, the if condition should be negated.

But even so, typeof(IRepository<>).IsAssignableFrom (repository) only yields true if repository == typeof (IRepository<>). Which is probably not what you meant.

Also, in general, do you think that continues are easier to read than if blocks that span more than one line? (This is a serious question.)

Francois Tanguay

Good catch. Ah the joy of of open generic types and interfaces. It is anything but predictable.

This little code snippet show well how much hidden complexity there is in any amount of code.

What seems elegant at first, end up being anything but it.

On a side note, there is plenty of room for some LINQification.

Ayende Rahien


No, I meant to have IRepository -> RepositoryImpl

Why do you think this is a bad approach?

Ayende Rahien


Yes, == false is my own personal idiosyncrasy

Ayende Rahien


Yes, it should be negated, but the exact semantics of how to do that escape me. Open generics types are a wholly mess.

Ayende Rahien

Yes, it is really annoying:

Type repository = typeof(NHRepository<>);

typeof(IRepository<>).IsAssignableFrom (repository) - false

repository.IsAssignableFrom (typeof(IRepository<>)) - false

mixing a lot of GetGenericTypeDefinition - false

Ayende Rahien


Yes, good catch. A case of two bugs masking each other :-)

The real code it is only a single line, I broke it up for presentation

herr ziffer

A serif font would improve it.

Francois Tanguay

You need to go through all interfaces of the type, get the GenericTypeDefinition of the Generic Interfaces as once implemented, they are closed and then do equality comparison




Colin Jack

How would this approach handle the situations where multiple entities do implement the same interface.

Ayende Rahien

That is why you are handing it a list of domain assemblies.

It doesn't make sense for entities to implement the same domain interface. Domain interface are not generic in nature.

Colin Jack

"It doesn't make sense for entities to implement the same domain interface. Domain interface are not generic in nature."

Not saying they are generic but you might have IAsset which can be implemented by multiple mapped classes. Quite rare I guess though, and you could deal with those cases differently.

Francois Tanguay

@Coliln: Funny, I have exactly that. ITrade and IAsset are implemented by multiple domain classes...

public class Bond : IAsset {}

public class Derivative : ITrade {}

public class SwapLeg : IAsset {}

public class BondTrade : ITrade {}

Colin Jack


Yeah, thinking after I posted it really isn't all that rare, for example we have multiple types of fee recipient so we have IFeeRecipient.


I often use C(#) embedded statements, and believe it´s a great combination with C(#) powefull operators (of course, always trying to preserve readability).

If it was VB we were ended with 2 times the lines of code, may be.

I just don´t like to replace (!) with (== false), because its more verbose. But I believe CSC.exe handle as they were the same instruction, so it´s just a preference. Isn´t it?

Ayende Rahien


You convinced me. I changed that to a predicate, so you can control the logic externally


I know many people don´t like embedded statements, here is an example, it doesn´t have any curly braces in the method´s body but does a lot of things. I´ve wrote it, believe it´s elegant:

    public object Get(string baseKey, TimeSpan expiration, CacheItemGetter getCacheItem)


        if (string.IsNullOrEmpty(baseKey))

            throw new ArgumentException("baseKey");

        if (getCacheItem == null)

            throw new ArgumentNullException("getCacheItem");

        object cacheItem = null;

        lock (BuildFullCacheItemKey(baseKey))

            if ((cacheItem = Get(baseKey)) == null && (cacheItem = getCacheItem()) != null)

                Add(baseKey, expiration, cacheItem);

        return cacheItem;

Pat Gannon

The cyclomatic complexity of this method is a little high for my tastes, due to the double-nested control structure (an 'if' in a 'foreach' in a 'foreach'). I would have broken Register up into two methods - probably a public Register and a private RegisterInterfaces that encapsulates the inner 'foreach' and the 'if'.

J.P. Hamilton

You could make that way nastier by using LINQ to Objects

Comment preview

Comments have been closed on this topic.


  1. The worker pattern - about one day from now

There are posts all the way to May 30, 2016


  1. The design of RavenDB 4.0 (14):
    26 May 2016 - The client side
  2. RavenDB 3.5 whirl wind tour (14):
    25 May 2016 - Got anything to declare, ya smuggler?
  3. Tasks for the new comer (2):
    15 Apr 2016 - Quartz.NET with RavenDB
  4. Code through the looking glass (5):
    18 Mar 2016 - And a linear search to rule them
  5. Find the bug (8):
    29 Feb 2016 - When you can't rely on your own identity
View all series


Main feed Feed Stats
Comments feed   Comments Feed Stats