Ayende @ Rahien

Stephan Hoekstra commented on Limit your abstractions: Refactoring toward reduced abstractions

Sun, 08 Apr 2012 12:22:32 GMT

Ok, thanks, I think I get it.

Ayende Rahien commented on Limit your abstractions: Refactoring toward reduced abstractions

Sun, 08 Apr 2012 12:15:09 GMT

Stephan, No, I am saying, SRP is orthogonal to the architecture that you are using. My point about SRP is that if the controller has many methods & actions, that is likely to be because of a SRP issue, not related to the architecture. And you shouldn't HAVE that many dependencies that make it complex to handle a single class in the first place

Stephan Hoekstra commented on Limit your abstractions: Refactoring toward reduced abstractions

Sun, 08 Apr 2012 11:31:51 GMT

I find this confusing, because I was taught that the Single Responsibility Principle is (always) our friend. Am I misrepresenting you when I would say that in this case you are basically advocating: - the application is simple enough and it is more valuable to just have code which is easy to work with. And if that violates SRP, too bad - we will deal with that when and IF it becomes a practical problem? - you would only consider refactoring if managing the dependencies becomes so costly (in terms of time) that it makes sense to abstract them away? Just trying to understand, Thanks.

Ayende Rahien commented on Limit your abstractions: Refactoring toward reduced abstractions

Sun, 08 Apr 2012 11:14:18 GMT

Stephan, It might be a problem from SRP point of view, but it wouldn't be a problem from any other aspect. It is still just as easy to work with the codebase, and just as easy to modify and change.

Stephan Hoekstra commented on Limit your abstractions: Refactoring toward reduced abstractions

Sun, 08 Apr 2012 11:09:43 GMT

Well, I am sure that in a real life scenario, CargoAdminController would have more functionality than just Register and BookNewCargo, and it would probably have more action methods and Commands (and Queries, etc.) Just because CargoAdminController is the logical place for such things, and real scenarios are always more complex than such example cases. You're right, I did overlook the fact that you do not apply DI for the commands in your example, so you would not have a gigantic constructor signature. But regardless of how you instantiate these objects, be it via a container framework or "new-ing them up", isn't it true that in both cases, if you have a lot of different command classes that you rely on, then you have a lot of dependencies (which is something you don't want - please correct me if I am wrong). I suppose I should rephrase my question: Let's say hypothetically, that your example controller grew over time, a number of action methods were added and you ended up with many more (dependencies on) commands and queries... Would you consider that to be a problem? Is that something you would be ok with, or would you refactor? If you would, then I'm curious to know how.

Ayende Rahien commented on Limit your abstractions: Refactoring toward reduced abstractions

Sun, 08 Apr 2012 10:08:22 GMT

Stephan, Why would this happen? Look at the code, I am instantiating and creating the command directly, it isn't a dependency that you get in the ctor.

Stephan Hoekstra commented on Limit your abstractions: Refactoring toward reduced abstractions

Fri, 06 Apr 2012 14:01:01 GMT

@Ayende, I have a question about your example of Controllers working with Commands and Queries. - I can imagine that if you have a complex controller which does a lot of things, you would end up having a lot of dependencies. Things can get crazy... public WorkaholicController( RecoverPassword recoverPasswordCommand, CreateUser createUserCommand, LoginUser loginUserCommand, UserOverViewQuery userOverviewQuery, ResetAllPasswords resetAllPasswordsTask, ... ...) //etcetera, credits for the example to my colleague Rodi ;) I suppose that would be a bad thing (right? having a lot of dependencies is not something you want.) The question is: I was wondering how you deal with that in practice? Refactor it into several smaller controllers? Compose the Commands into higher level commands that have subcommands? Is it even a problem?

henginy commented on Limit your abstractions: Refactoring toward reduced abstractions

Thu, 23 Feb 2012 15:45:16 GMT

Nice ideas. I would like to ask how .NET 4.0 Tasks would fit in this architecture (Tasks & Commands)? It seems to me they would somewhat overlap. In summary, what do you think of following the same approach with .NET Tasks?

meisinger commented on Limit your abstractions: Refactoring toward reduced abstractions

Thu, 23 Feb 2012 06:10:36 GMT

wow... this conversation has taken a turn for the worse! i never thought that i would ever see the day that someone brings up sql injection to the guy who contributed tons of code to NHibernate and created RavenDB i am actually surprised there was even a response @Ayende... you are a much more patient person than i am. also let me know when RavenDB has prepared statements... that is all

Ivan commented on Limit your abstractions: Refactoring toward reduced abstractions

Thu, 23 Feb 2012 01:14:06 GMT

Hey, guys, there's ORM between domain code and actual SQL. It handles this kind of things and much more. What are you talking about here at all?

Erlend commented on Limit your abstractions: Refactoring toward reduced abstractions

Wed, 22 Feb 2012 13:56:25 GMT

Imho, you missed Dan's point. By reducing TrackingId to a string, your code basically ends up saying that a tracking id can be any string. Your code stopped caring about the contents of it. Instead of removing TrackingId, you should augment it to contain any validation rules you have, so that whenever you have a TrackingId-object it's calid according to your domain rules. Thus anywhere else in your code, you don't have to worry about it's contents. As long as you have a TrackingId, you know it's valid. The same holds for UnLocode. Why on earth would you want to send strings around, when you can send proper objects... like DateTime.

lindo commented on Limit your abstractions: Refactoring toward reduced abstractions

Wed, 22 Feb 2012 12:44:39 GMT

I was merely reacting to your comments re SQLi being a non-issue in this day & age. Extrapolated the VULNERABLE from that. Perhaps loss in translation is a little to blame for my assumption?? Context is important re security AND education. Reading this post alone does not make it clear that this code is part of a specific, open project nor the context in which some inputs are derived/born. Looks like originUnlocode and destinationUnlocode are more of a worry, upon inspection. Im not versed in c# fortunately.

Ayende Rahien commented on Limit your abstractions: Refactoring toward reduced abstractions

Wed, 22 Feb 2012 12:23:11 GMT

Thiago, I have _absolutely_ no idea how the issue of SQL injection came up. But yes, with NHibernate, you don't have to worry about SQL Injections

Thiago commented on Limit your abstractions: Refactoring toward reduced abstractions

Wed, 22 Feb 2012 12:20:21 GMT

Doesn't nhibernate takes care of sql injection issues? I'm not very familiar with it, but googling arround shows me that it takes care of sql injection, as long as you let nhibernate create the queries for you.

Ayende Rahien commented on Limit your abstractions: Refactoring toward reduced abstractions

Wed, 22 Feb 2012 12:12:09 GMT

Psiinon, HUH?! What on earth are you talking about? Where do you even see the discussion on prepared statements vs. sql concat? Where on EARTH do you see a possible sql injection in the code above.

Psiinon commented on Limit your abstractions: Refactoring toward reduced abstractions

Wed, 22 Feb 2012 12:04:00 GMT

Hi Ayende, ok, I will agree that your code looks ok as it stands. However I would still always recommend that you use prepared statements when accessing databases. You never know how your code is going to be used in the future, or whos going to copy it and reuse it for some other purpose. You are writing are popular blog, and so I think you should always demonstrate best practice. Someone could easily copy your code and then modify it for their own purposes, introducing a real SQL injection vulnerability.

Ayende Rahien commented on Limit your abstractions: Refactoring toward reduced abstractions

Wed, 22 Feb 2012 11:36:22 GMT

Lindo, Do take a moment to relax. Again, the codebase for this blog is openly available. Feel free to try to find a SQL injection issue in here. And I do hope that you aren't really going to tell me that this code adds anything to security, right? http://code.google.com/p/ndddsample/source/browse/trunk/src/NDDDSample/app/domain/NDDDSample.Domain/Model/Cargos/TrackingId.cs And that is leaving aside the fact that you are talking about a value that is _generated by the application_. Not user input

Ayende Rahien commented on Limit your abstractions: Refactoring toward reduced abstractions

Wed, 22 Feb 2012 11:34:20 GMT

Psiinon, _I_ am the one who is going to create this value. Trying to protect me from my own SQL injection doesn't seems insane to you? You know what, the code for this blog is public, go ahead and try to find a SQL injection here. I am taking security very seriously, but do tell me how this thing helps security in ANY way: http://code.google.com/p/ndddsample/source/browse/trunk/src/NDDDSample/app/domain/NDDDSample.Domain/Model/Cargos/TrackingId.cs

lindo commented on Limit your abstractions: Refactoring toward reduced abstractions

Wed, 22 Feb 2012 10:10:18 GMT

Lulz. Ayende's own words " If you don't care what is going on, you are going to create piss poor software."...while defending the PISS POOR VULNERABLE SOFTWARE he writes. Its not about you, Ayende, your softwares userbase are subject to attack due to you totally missing the point when it comes to security. But if you want to prove your vulnerabilites are negligable in this day & age, let us know where to find production instances of your almighty software.

Psiinon commented on Limit your abstractions: Refactoring toward reduced abstractions

Wed, 22 Feb 2012 09:54:39 GMT

Ayende, yes the fact Dan is worried about SQL injection probably tells you a lot about the projects he works with. Real world ones. Ones whose owners dont want them to get compromised. I'm always amazed that SQL injection attacks can still work as there are simple defensive measures that can prevent them, but its still the number one web application security risk in the OWASP Top 10. Your attitude explains how they still occur. Very sad :(

JohnWilander commented on Limit your abstractions: Refactoring toward reduced abstractions

Wed, 22 Feb 2012 09:17:18 GMT

Ayende Rahien, your reply "Feel free to write all of those validations in your code. I have actual business value to deliver." actually chocks me. I always say developers really care about quality and robustness. Ignoring well-known security issues will not get you far in this era of software development, IMHO.

Ayende Rahien commented on Limit your abstractions: Refactoring toward reduced abstractions

Wed, 22 Feb 2012 09:14:23 GMT

Dan, The mere fact that you need to worry about data such as: ' OR 1= 1 Tells me a lot about the kind of projects you work with. I wish you all the best, and enjoy Boby's Tables visit: http://xkcd.com/327/ Seriously, you really care about those sort of things? And you call them security issues? In this day & age?

Dan Bergh Johnsson commented on Limit your abstractions: Refactoring toward reduced abstractions

Wed, 22 Feb 2012 08:49:30 GMT

Ayende. Ahh, I see. I stand corrected. I assumed that mitigation of business risks such as security was something your stakeholders cared about. Now I understand they do not, so I assume it is some kind of intranet application. Well, make sure your user interface never leaves the safe harbor of your intranet. The day it becomes publicly accessible, LulzSec will love you.

Trystan commented on Limit your abstractions: Refactoring toward reduced abstractions

Tue, 21 Feb 2012 22:39:22 GMT

So I'm guessing that the commands expose their dependencies with properties and whatever implements the ExecuteCommand method uses setter injection to make sure the Command has all of it's dependencies? That means your command executer (domain objects? controllers? other commands?) needs all the dependencies. This would make it easier to implement requirements that depend on where something is being done from ("send an email when doing X from page Y but send a text message when doing X from page Z") but it still seems a little odd to me. I think I need a more complete example of commands that rely on external services like email, legacy systems, or web services to really get it. I look forward to the upcoming posts!

Dmitry commented on Limit your abstractions: Refactoring toward reduced abstractions

Tue, 21 Feb 2012 22:38:13 GMT

@Oren, I try to avoid property injection whenever possible. It makes the classes more brittle because in a multi-developer project it is very likely for someone to add a that does will not be resolved by the container. Also, it exposes dependencies in the interfaces if they are used.

Lev Gorodinski commented on Limit your abstractions: Refactoring toward reduced abstractions

Tue, 21 Feb 2012 22:28:43 GMT

"No, it doesn't. Assuming ASP.NET MVC, you are working at a FAR higher level than raw HTTP." Sure, ASP.NET MVC itself exposes a far higher level of abstraction than HTTP, nor does it deal directly with HTTP. However the point is still that the framework lives at an application boundary and from a birds eye view, HTTP messages are mapped to application code. The level of abstraction that the ASP.NET MVC framework connects with internally doesn't matter to clients of the framework. If ASP.NET MVC was changed to listen to sockets directly, or even HTTP.sys, the clients wouldn't care. "Trying to pretend that it does and then add abstractions adds needless complexity." Adding needless abstractions adds needless complexity, but I don't see the relationship between "pretending" that ASP.NET MVC operates upon raw HTTP messages and adding abstractions. "It split things up on a SRP lines. You have independent commands, not complex set of interconnected services." Yes of course "complex set of interconnected services" sounds bad. If instead you replace that with "service" it no longer sounds that bad :). In fact, I think designing a framework for executing commands from a controller seems like a needless abstraction where you can just delegate directly to a facade (service, or repository). If the proposed command model is SRP then so is a facade. The controller's single responsibility is to map external client activity to facades, where facades do whatever needs to be done, be it getting some data from a database, placing a message on a queue, etc. The facade implements the core logic of the application, the controller implements interop.

Ayende Rahien commented on Limit your abstractions: Refactoring toward reduced abstractions

Tue, 21 Feb 2012 21:59:31 GMT

Trystan, Commands don't execute themselves, they are being executed by the command executer

Ayende Rahien commented on Limit your abstractions: Refactoring toward reduced abstractions

Tue, 21 Feb 2012 21:58:41 GMT

Dan, Feel free to write all of those validations in your code. I have actual business value to deliver.

Ayende Rahien commented on Limit your abstractions: Refactoring toward reduced abstractions

Tue, 21 Feb 2012 21:57:42 GMT

> It maps raw HTTP messages to Java/C#/Whatever No, it doesn't. Assuming ASP.NET MVC, you are working at a FAR higher level than raw HTTP. Trying to pretend that it does and then add abstractions adds needless complexity. > Furthermore, the proposed command based solution is no different than delegating to a repository or service. Um, not really. It split things up on a SRP lines. You have independent commands, not complex set of interconnected services.

Ayende Rahien commented on Limit your abstractions: Refactoring toward reduced abstractions

Tue, 21 Feb 2012 21:55:20 GMT

Dmitry, Nothing prevents you from injecting the dependencies into the command, see the previous answer about InjectProperties. I don't like the syntax when you have an additional lambda.