Ayende @ Rahien

J.P. Hamilton commented on Elegant Code

Thu, 27 Mar 2008 17:03:40 GMT

You could make that way nastier by using LINQ to Objects

Pat Gannon commented on Elegant Code

Thu, 27 Mar 2008 16:24:29 GMT

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

Bruno commented on Elegant Code

Thu, 27 Mar 2008 15:31:38 GMT

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; }

Ayende Rahien commented on Elegant Code

Thu, 27 Mar 2008 15:18:31 GMT

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

Bruno commented on Elegant Code

Thu, 27 Mar 2008 15:13:53 GMT

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?

Colin Jack commented on Elegant Code

Thu, 27 Mar 2008 15:11:35 GMT

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

Francois Tanguay commented on Elegant Code

Thu, 27 Mar 2008 15:03:21 GMT

@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 commented on Elegant Code

Thu, 27 Mar 2008 14:22:41 GMT

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

Ayende Rahien commented on Elegant Code

Thu, 27 Mar 2008 14:12:26 GMT

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 commented on Elegant Code

Thu, 27 Mar 2008 14:07:31 GMT

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

Francois Tanguay commented on Elegant Code

Thu, 27 Mar 2008 13:50:09 GMT

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>.GetInterfaces()[0].GetGenericTypeDefinition());

herr ziffer commented on Elegant Code

Thu, 27 Mar 2008 13:45:45 GMT

A serif font would improve it.

Ayende Rahien commented on Elegant Code

Thu, 27 Mar 2008 13:42:45 GMT

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

Ayende Rahien commented on Elegant Code

Thu, 27 Mar 2008 13:41:42 GMT

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 commented on Elegant Code

Thu, 27 Mar 2008 13:40:07 GMT

Francois, 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 commented on Elegant Code

Thu, 27 Mar 2008 13:27:06 GMT

Grimace, Yes, == false is my own personal idiosyncrasy

Ayende Rahien commented on Elegant Code

Thu, 27 Mar 2008 13:13:56 GMT

Bogan, No, I meant to have IRepository -> RepositoryImpl Why do you think this is a bad approach?

Francois Tanguay commented on Elegant Code

Thu, 27 Mar 2008 12:50:55 GMT

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.

Fabian Schmied commented on Elegant Code

Thu, 27 Mar 2008 12:20:35 GMT

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 commented on Elegant Code

Thu, 27 Mar 2008 12:02:58 GMT

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

Grimace of Despair commented on Elegant Code

Thu, 27 Mar 2008 11:38:24 GMT

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

json commented on Elegant Code

Thu, 27 Mar 2008 11:27:35 GMT

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

Bogdan Pietroiu commented on Elegant Code

Thu, 27 Mar 2008 09:40:20 GMT

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