Hi Ayende,

When you coded

Component.For(typeof(IRepository<>).MakeGenericType(interfaceType)).ImplementedBy(repository.MakeGenericType(interfaceType))

you meant

Component.For(typeof(IRepository<>).MakeGenericType(interfaceType)).ImplementedBy(repository.MakeGenericType(mappedClass))

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)

..or...

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

Personally I prefer 4 chars deep indents

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

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

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.)

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.

Bogan,

No, I meant to have IRepository<IParent> -> RepositoryImpl<IParent>

Why do you think this is a bad approach?

Grimace,

Yes, == false is my own personal idiosyncrasy

Francois,

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

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

Fabian,

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

A serif font would improve it.

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

Assert.Equal(

typeof(IRepository<>),

typeof<NHibernateRepository<T>>.GetInterfaces()[0].GetGenericTypeDefinition());

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

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.

"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.

@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 {}

@Francois

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?

Guys,

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;

    }

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'.

You could make that way nastier by using LINQ to Objects