Ayende @ Rahien

Mats Helander commented on Reviewing NerdDinner

Fri, 07 Aug 2009 15:22:28 GMT

>So a single class requiring something that you cannot provide by default render the whole issue mote. <
No, I don't agree. If I have just the logger, I probably would consider dragging a new (DI) framework into the mix as a bit overkill/unnecessary overhead. In fact, in your recent post [ayende.com/.../do-you-need-a-framework.aspx](http://ayende.com/Blog/archive/2009/08/05/do-you-need-a-framework.aspx) you excellently sum up my view here, so for reference, I refer you to yourself ;-) Now, as you have more and more stuff _like_ the logger, at some point the DI framework will start paying for itself. It seems you consider yourself to be at that point at a somewhat earlier stage than I do, but I think that's all we're arguing about in _that_ respect. (Please note that I don't see any logical disconnect in you still agreeing with yourself in that post but also thinking the overhead for a DI framework is small enough to warrant its inclusion more often than I do. We view some costs differently, is all) The original argument, however, was about whether it was for some reason bad to provide some sensible default collaborators as part of the default implementation of an interface. And here I remain unconvinced, even though Dmitry's latest comment managed to plant a nagging seed of doubt in me there... >Yes, even around advices & interceptors. :-O I'll have to go check how the heck they pull off that magic then...cool! :-) /Mats

Ayende Rahien commented on Reviewing NerdDinner

Sat, 01 Aug 2009 13:55:45 GMT

Mats, Yes, even around advices & interceptors. > so the sensible default for it is an empty list Which render it useless So a single class requiring something that you cannot provide by default render the whole issue mote. At that point, why even bother?

Mats Helander commented on Reviewing NerdDinner

Sat, 01 Aug 2009 11:49:02 GMT

>Not really, I can think of other examples as well. public Logger(ILoggingDest[] destinations)<
But the Logger doesn't depend on having any destinations at all, so the sensible default for it is an empty list. I definitely agree that this is an example of something that could well be configured by the DI Framework. I'm talking about when an object needs some collaborators to function and when there are sensible default such collaborators to be had. /Mats

Mats Helander commented on Reviewing NerdDinner

Sat, 01 Aug 2009 11:45:54 GMT

>Take a look at Dynamic Proxy, for example. There is _no_ reflection involved in generating a method call. Including calling interceptors or calling the actual method<
Even for around interceptors/advice? /Mats

Ayende Rahien commented on Reviewing NerdDinner

Sat, 01 Aug 2009 10:47:47 GMT

Mats, Take a look at Dynamic Proxy, for example. There is _no_ reflection involved in generating a method call. Including calling interceptors or calling the actual method. > re: con str Not really, I can think of other examples as well. public Logger(ILoggingDest[] destinations) Now you need to find all the destinations in the application. This is a fixed list, but I _don't_ want to make this logic in a collaborator. But I don't think that we can agree on that.

Mats Helander commented on Reviewing NerdDinner

Sat, 01 Aug 2009 10:44:33 GMT

>Most AOP systems in .Net are working using dynamic code generation. In other words, they are as fast as anything that you can write.<
Sure they generate code, but they generate glue code that delegates to the weaver that passes the packed up method to the interceptors that will use reflection to call them. >Really? My logger impl. needs a connection string, not everything that uses FormAuth needs a connection string because a collaborator of FormAuth needs that.<
So that connection string is then not a default value but a customization, and thus a great case for a DI framework! No argument there! But that doesn't mean that a sensible default value (in this case probably null/empty string) couldn't be encoded into the class. /Mats

Mats Helander commented on Reviewing NerdDinner

Sat, 01 Aug 2009 10:40:03 GMT

>Yuck, isn't that _way way way_ too much repetition to write? Hehe, btw, you should see the scope of the bloat I'm actually writing...:-P I adhere strictly to the following rule: 1) I *always* bind to interfaces. 2) Each interface has an abstract Base implementation (with the common reusables in it) 3) Then there is the (concrete) Default implementation, which inherits from the base class (and has sensible defaults for the members that were left abstract in the Base) 4) Specializations can inherit from either the Default or the Base (if the defaults are not at all useful) 5) Often there will be a mixin as well. This will then contain the actual implementation from the Base class, and the Base class will only delegate to the mixin. A class that already inherits another Base class can then also use the mixin to get the behavior. I normally wouldn't write in this bloated way for a client, mind you. This is the way I code my own projects for _fun_! ;-) To me, the fun is in getting it right, even if it often takes lots more code and it may not be cost effective :-P But whenever I keep extending a code base that has grown beyond some complexity, and it follows this formula, I have to say I love it - and whenever I cheat on the formula I seem to come to regret it... /Mats

Ayende Rahien commented on Reviewing NerdDinner

Sat, 01 Aug 2009 10:32:23 GMT

Mats, > I don't want to pay the framework overhead (reflection, basically) Most AOP systems in .Net are working using dynamic code generation. In other words, they are as fast as anything that you can write. > Nope, you just go into FormAuth and change its default collaborators! :-) Really? My logger impl. needs a connection string, not everything that uses FormAuth needs a connection string because a collaborator of FormAuth needs that.

Mats Helander commented on Reviewing NerdDinner

Sat, 01 Aug 2009 10:24:48 GMT

>Yuck, isn't that _way way way_ too much repetition to write? Yes, compared to having built-in support for mixins in the language (when will C# get this??) but there is no repetition of _logic_, which is the important part. I'm doing it manually rather than using an AOP framework because it is performance critical code and I don't want to pay the framework overhead (reflection, basically) and I'm doing it at all because I absolutely don't want to repeat any logic...so I'm using this whenever I have already used up the inheritance option. Yes this would be a perfect case for code generation ;-) I will review the RSB code to see what I find, but, >A good example would be when I need to change FormAuth to use something else that it requires. Now I need to go and change it everywhere that create a FormAuth, and that is created everywhere that has a dependency on that.<
Nope, you just go into FormAuth and change its default collaborators! :-) That's the beauty. What's that, FormAuth not your class? ;-) Well that's why we should always encapsulate our external dependencies, right? Adapter time. /Mats

Ayende Rahien commented on Reviewing NerdDinner

Sat, 01 Aug 2009 10:10:29 GMT

Mats, > But why would it not be the proper concern of the object to know its null substitution values Because this create a _very_ hard to change object graphs. A good example would be when I need to change FormAuth to use something else that it requires. Now I need to go and change it everywhere that create a FormAuth, and that is created everywhere that has a dependency on that. That is even leaving aside issues like life cycle management. Creation of a dependant object is often a complex issue, and not something that should be the responsability of a class. > I'm not sure that the overall design of my apps looks so different from yours I am pretty sure that there is. Did you review the RSB code base? It is not a slur, it is just different methods of approaching the problem. And no, having DI and baking the assumption of DI into the architecture are two different things. > re: Mixins Yuck, isn't that _way way way_ too much repetition to write?

Mats Helander commented on Reviewing NerdDinner

Sat, 01 Aug 2009 09:06:42 GMT

Ayende, >How are you doing this? The obvious (cumbersome :-P) way :-) public interface IMyFace { string ShowIt(double input); int CalcIt(double input); } public class MyClass : IMyFace { public MyClass() { myFaceMixin = new MyFaceMixin(this); } private IMyFace myFaceMixin; public string ShowIt(double input) { return this.myFaceMixin.ShowIt(input); } public int CalcIt(double input) { return this.myFaceMixin.CalcIt(input); } } public class MyFaceMixin : IMyFace { public MyFaceMixin(IMyFace target) { this.target = target; } private IMyFace target; public string ShowIt(double input) { //Note that we must call target.CalcIt(), not this.CalcIt(). //This is so the target gets a chance to //override the implementation and intercept the call return "Calculation Result: " + target.CalcIt(input).ToString(); } public int CalcIt(double input) { return Convert.ToInt32( input ) * 42; // << Teh Usefulness } }

Mats Helander commented on Reviewing NerdDinner

Sat, 01 Aug 2009 08:56:36 GMT

Dmitry, But why would it not be the proper concern of the object to know its null substitution values (which is what I think you use in your example, rather than a default value)? The same question, of course, goes for default values. Ayende, I'm not sure that the overall design of my apps looks so different from yours, just that I don't often see the need to use a DI container for specifying the default configurations. And most certainly there are cases when no sensible default collaborators present themselves (for example, why would the NHibernatePersistenceService be the default IPersistenceService....there it would probably be better to rely on a DI Framework or a Service Locator to use a configuration to inject the appropriate persistence service) but in cases where obvious such defaults exist - such as in the ND example - I just don't see the drawback...

Ayende Rahien commented on Reviewing NerdDinner

Sat, 01 Aug 2009 06:34:11 GMT

> writing and using mixins etc without using any actual AOP framework for the weaving How are you doing this? > Then I need to have designed for this to work Mats, The issue here is not whatever it can be made to work or not, the issue is one of overall design. In recent years, all my apps has been using a container, to the point where I consider a container to be as necessary as System.String. Once you accept a container as a given, your entire approach for designing a system goes through a fairly rapid transformation, because a container make things such as complex compositions so much easier. Again, I can only point out to RSB for an example of such a system.

Dmitry commented on Reviewing NerdDinner

Sat, 01 Aug 2009 00:11:19 GMT

@Mats, Would you write ADO code like this. var value = reader["field"] != DBNull.Value ? reader.GetString("field") : "Some default value"; Or would you rather have a DB trigger/ORM interceptor/etc replacing NULLs with "Some default value", assuming the requirement makes sense? There are 2 issues here: separation of concerns and magic values.

Mats Helander commented on Reviewing NerdDinner

Fri, 31 Jul 2009 23:30:54 GMT

Ayende, >You are standing in the wrong place :-) Haha :-) I enjoy knowing that I stand in a different ring corner from you sometimes, since usually I find myself agreeing with almost everything you say. Now, if we had been talking about, say, an O/R Mapper, I would agree that it would be mad to have your app working fine both with and without it....when it comes to an AOP framework...I'm not so totally sure anymore - in fact I tend to do more and more "manual AOP" these days (writing and using mixins etc without using any actual AOP framework for the weaving). When it comes to default configurations, this I find so easy to do manually that the DI frameworks, frankly, mostly seem to add weight and come in my way. But then, that is until I start wanting to support custom configurations, at which point specifying those with the help of a DI framework makes all the sense in the world. Then I need to have designed for this to work, but it seems to me that the ND ctor should work fine in such scenarios. But then, that's just me I guess :-) @Fredy >At least to me, the null parameters meaning 'default' is what strikes me as odd, Am I wrong? Ok, I buy that all magic values are inherently bad, including having null mean "use default"...only that this is so common I hardly see it as a magic value anymore. But in principle I do buy your point. @Dmitry I'm not sure why creating the objects there is better...with the ND approach you get a comfy way of passing only the collaborators you want to customize and pass null to the rest to get default collaborators for them. To me that's a win. And in no way do I see this as similar to using switch statements instead of proper OO polymorphism ;-) Happy hacking! /Mats

Ayende Rahien commented on Reviewing NerdDinner

Fri, 31 Jul 2009 21:39:24 GMT

Mats, > Where I stand, the app should be able to stand on its own, working with useful default configurations, without involving any DI container/configuration framework You are standing in the wrong place :-) More seriously, containers take away a LOT of the comlexities inherit in composing applications. Trying to do it by hand is just waste at this point in time. Take a look at some apps / frameworks that were explicitly written with an IoC in mind. Rhino Service Bus as a good example.

Dmitry commented on Reviewing NerdDinner

Fri, 31 Jul 2009 19:11:18 GMT

@Mats, I still do not understand why do you need this additional logic, even if you are not planning to extend/refactor the application. Why not make the code easier, such as: public AccountController : this(new FormsAuthentication(), new MembershipService()) { } * I know it is not how you instantiate the objects but it's just to make a point. You can always use if/switch/?/?? operators instead of using polymorphism or method overloading as well but why? The point is to have code that is well separated and easy to understand.

Fredy Treboux commented on Reviewing NerdDinner

Fri, 31 Jul 2009 18:34:41 GMT

@Mats I don't think anyone is saying that, given the constraints you want to apply, it's wrong to provide default implementations of the dependencies on the same class. What you don't want to do however, is tie the same constructor you *could* use for injection with those implementations... the common approach is to use another constructor that creates the dependecies and call the DI constructor. At least to me, the null parameters meaning 'default' is what strikes me as odd, Am I wrong?

Mats Helander commented on Reviewing NerdDinner

Fri, 31 Jul 2009 17:05:37 GMT

"This logic would be totally useless if a DI container was used." Right, but for the case when the DI container is _not_ used...? (But again, it can be, for custom configuration) If that is a completely unrealistic scenario for you, then I buy what you are saying in that context, but for me that's a very realistic scenario. /Mats

Dmitry commented on Reviewing NerdDinner

Fri, 31 Jul 2009 16:55:54 GMT

@Thomas Eyde, You are absolutely correct. But I was talking about this example where IFormsAuthentication and IMembershipService are interfaces.

Dmitry commented on Reviewing NerdDinner

Fri, 31 Jul 2009 16:53:41 GMT

@Mats, The DI (biggest) constructor should only set the member variables of the initialized object to the provided values. The additional logic that handles "nulls" makes the constructor ugly because now it tries to do more than necessary. This logic would be totally useless if a DI container was used. Dmitry

Mats Helander commented on Reviewing NerdDinner

Fri, 31 Jul 2009 15:01:27 GMT

Ayende, Thank you for your reply, I'm still not sure I'm with you/agree, though I think I understand what you are saying now. I think it may come down to different perspectives and preferences...concretely, I start out by disagreeing with: "Poor Man's IoC isn't great, but it is a good solution if you can't use real IoC" I don't want to paint you into some corner where you have to defend a lot that you may not have meant based on a quick comment in trying to explain something to me, but I think I really have a different perspective here. Where I stand, the app should be able to stand on its own, working with useful default configurations, without involving any DI container/configuration framework. Then it should preferably be written so that it allows further configuration via such frameworks, but it should not require them, not for custom configuration and absolutely not for standard setup/initialization. For a concrete class to have built-in dependencies to its default concrete collaborators I see as no problem at all - I see that in fact as good practice, based on what I said above that you should be able to start the app up without configuring it and have it run in a standard/default mode. A DI framework should be able to override the use of the default collaborators, of course, but you should be able to override via "manual" configuring code as well of course (easily, not via reflection or weird special methods...just passing what you like to the constructors or setting properties should be just as easy to do for the developer as for a DI framework). In this perspective, having a class know of its default collaborators is thus not to scatter responsibilities around - it is placing them where they belong. "The pattern shown in ND is of a ctor passing null to another ctor to rely on a default implementation in that ctor." Yup. "It is ugly." On this we disagree. I think it is pretty. "It means that there is more code, conditionals and that changing something isn't as easy as it should." This of course would make me agree it was ugly if you could make me see that this were true. But why would this be the case when we are talking about specifying _default_ implementations for the collaborators an object depends on? And isn't the alternative if you don't want to specify default collaborators in the code to totally rely on a DI framework to get the app up and running? I think perhaps our main point of disagreement is that I don't think it is neat for an object to have a "dependency" on an outside configuring process to be able to do any work at all - to me that's much worse than a default implementation of something having built-in dependencies to other default implementations of its collaborators (which should probably reside in the same assembly) because, again, I see this as no real problem at all but almost verging on a feature of OO (in essence, it is the abstract factory pattern!). To try to become a bit more concrete - do you have any actual examples of how passing null to the ctor which knows how to then use a default implementation makes it so that "changing something isn't as easy as it should."? Remember, I _want_ to change which default collaborator to use inside the class, not in a config file because in my perspective that is the right place to do it. And it is just one change in one place, regardless of if it is the class or in a config file. Yes it requires a recompile, but remember that's (almost) how I would want to have it for changing _default_ collaborators (at least I see it as no cost at all, and I might even get a compilation error out of it if I did something stupid). "More code" - Yes (very little) but fewer lines of configuration. "More conditionals" - Why? Wouldn't that conditionality just be moved somewhere else (out of the class where at least I think it belongs)? As always, Thank You for your utterly entertaining, thought-provoking and enlightening blog! /Mats

Ayende Rahien commented on Reviewing NerdDinner

Fri, 31 Jul 2009 14:10:47 GMT

Mats, The problem is that there are well known conventions for doing things like that. Poor Man's IoC isn't great, but it is a good solution if you can't use real IoC. The pattern shown in ND is of a ctor passing null to another ctor to rely on a default implementation in that ctor. It is ugly. It means that there is more code, conditionals and that changing something isn't as easy as it should. Same thing for the Sochipath's IoC. It is scattering responsibilities everywhere. The biggest ctor should be concerned with just getting the parameters to fields, and initalizing the class. It should not make decisions about default implementations.

Mats Helander commented on Reviewing NerdDinner

Fri, 31 Jul 2009 14:02:35 GMT

I don't understand what the problem is with the "Mad Man's DI" there. Dimitri, you obviously have an answer, but it goes whoosh over my head...could you expand a bit on what you are saying for the thicker of us? Jeremy, I don't see what's wrong with the Sociopath's DI either...guess that makes me a Mad Sociopath, huh? :-) Could you too please expand a bit on what you find wrong with your example? I'm not sure what part of the context I am missing....are these patterns you would _always_ consider bad, or is it just in the context of wanting to use some tool/framework that doesn't jive well with these setups? /Mats

Patrick commented on Reviewing NerdDinner

Fri, 31 Jul 2009 14:00:27 GMT

Just to follow up on what Phil said, I would personally like to see bits of code that you found to be well designed. Sometimes I get the feeling that the term "code review" ends up becoming "pointing out everything I don't like". And as a side note, I agreed with everything you said, especially about the unit test and the attribute saturation. The unit test seems to support the notion of "100% code coverage is the goal of testing" which makes me cry :( Thanks to NerdDinner folks for the sample app and to Oren for the feedback.

Dave commented on Reviewing NerdDinner

Fri, 31 Jul 2009 11:06:26 GMT

Unfortunately routing rules don't really work. Or I have to use about 100 hardcoded paths in each supported language. But without heavy modifcations you can't map the url '/profiel/registratie' to AccountController with action 'Register'. I can work around that with a ProfielController class, but we also have the same url's in English (/Profile/Register), German (/Profil/Registrierung), Spanish (/Perfil/registro) and French (/Profil/enregistrement). They all have to map to the Register action. But what about input differences (cultures) like date and number formats. What about metric and unit measurements (Feet vs Meter, Celsius vs Fahrenheit)? Should you display a french-only content page to a visitor who has selected the English language? System pages (like login, forgot password, etc) are automatically redirected to the url associated with the current language. We haven't spent 3 months only on writing a new routing module, we spent 3 months making MVC be able to cope with localization.. and than we spend 6 months on building a website upon our modified MVC framework. And even when you have an English only MVC website, it's bad practice to let presentation determine how to name your classes. You can't just change the url /Account/Create to /Account/Register. You actually have to refactor your code (rename the Create methods to Register) just because you want to make a presentation change.. where's the separation between presentation and code here? I hope this clarifies things a bit..

Marcel commented on Reviewing NerdDinner

Fri, 31 Jul 2009 10:16:12 GMT

When I was reading the part of "Cascading Deletes" I wondered if I missed something. Did quick research and Linq2SQL doesnt support this. I know in NHibernate its pretty simple, but for now seems ok to delete as they did.

Thomas Eyde commented on Reviewing NerdDinner

Fri, 31 Jul 2009 09:37:23 GMT

@Hacked, You could have split the file and moved them to a subfolder. Deleting one controller and one folder isn't that hard. @Dave, I think you can use routing to create your Dutch urls. I wish I thought of that earlier. No wait, I opted to code in Norwegian. That helps too, but the Norwegian code upset some developers, the Dutch included I guess :-)

Thomas Eyde commented on Reviewing NerdDinner

Fri, 31 Jul 2009 09:36:48 GMT

@Dmitry, There's no problem having concrete classes in a DI constructor when you don't need the interface. I know people differ on the subject, but coding against an abstraction doesn't necessarily mean abstract classes or interfaces. The real power is you state your dependencies in the constructor and let the DI container do the dirty work.

Bunter commented on Reviewing NerdDinner

Fri, 31 Jul 2009 09:13:08 GMT

Dave, have you ever heard of routing rules in ASP.NET MVC?? :) I seriously hope you did something other for three months than changing controller names and rewriting MVC, otherwise your time spent was utter waste.