locked
Where should I execute query using ToList() - in DAL, BLL or in controller? RRS feed

  • Question

  • User-1357109602 posted

    I have project DAL, BLL and Web. Where should I execute query using ToList()?

    Right now I execute query using ToList() in controller - is it ok? For example - from the beginning:

    DAL - my method in NotesRepository class:

        public IQueryable<Notes> GetAllNotes()
        {
            return context.Notes.Include(x => x.Comments).OrderByDescending(x => x.CreateDate);
        }



    BLL - my method in NotesService class:

        public IEnumerable<Notes> GetNotes()
        {
            return _unitOfWork.NotesRepository.GetAllNotes();
        }




    Web - my action in controller (execution query using ToList()):

        public ActionResult Index()
        {
            var notes = _notesService.GetNotes().ToList();
        
            return View(notes);
        }



    Wednesday, July 2, 2014 11:09 AM

Answers

  • User-760709272 posted

    People will differ in their opinions, but you should return a List from your repository.  Your repository is supposed to abstract your data storage from the business layer, and if you return IQueryable then you're not doing this.  If (in theory) you wanted to move from a data storage that didn't support IQueryable then you wouldn't be able to.  It also means your controller needs a connection to your data as it is only when ToList (or similar) is called that the command is actually executed, so your repository isn't acting like a repository at all, so if you wanted to put your business layer, or your data layer, behind a service etc you wouldn't be able to as the controll\bll and dll are strongly coupled to each other.

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Wednesday, July 2, 2014 12:37 PM

All replies

  • User-760709272 posted

    People will differ in their opinions, but you should return a List from your repository.  Your repository is supposed to abstract your data storage from the business layer, and if you return IQueryable then you're not doing this.  If (in theory) you wanted to move from a data storage that didn't support IQueryable then you wouldn't be able to.  It also means your controller needs a connection to your data as it is only when ToList (or similar) is called that the command is actually executed, so your repository isn't acting like a repository at all, so if you wanted to put your business layer, or your data layer, behind a service etc you wouldn't be able to as the controll\bll and dll are strongly coupled to each other.

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Wednesday, July 2, 2014 12:37 PM
  • User-1611549905 posted

    I'd beg to differ. Decoupling your DAL from your business layer and controller sounds like a nice idea in theory, but in practice it causes more problems than it solves. Either you'll lock yourself out of a whole lot of features of your ORM that you need for performance optimisation and other purposes (lazy versus eager loading, caching, transaction management, adding interception into your DAL to handle cross-cutting concerns, streaming tens of millions of objects taking up several gigabytes, and so on) or else you'll end up building something so complex and convoluted that it's riddled with bugs.

    As for swapping out your data source for something else, in most cases that's just architecture astronaut territory. Ditto putting your data layer or business layer behind a service. Effectively, you'll be optimising for problems that you're not likely to face at the expense of problems that you are.

    Tuesday, July 8, 2014 12:22 PM
  • User-760709272 posted

    If you need to keep transactions running all the way to your view then I'd say you have some architecture problems yourself.  The only real thing you're "locking yourself out of" in terms of ORM is probably lazy loading...but again, if your presentation makes the decisions on when your data layer should employ lazy loading then you have some architecture problems anyway.  It seems you might be of the opinion that exposing your data layer to your presentation is better to do simply because it is "easy" rather than thinking about proper separation of concerns.

    I was addressing the question of repositories specifically, I stand by that any class that returns a query isn't a repository, it is a query builder.  You already have LinqToSQL and EF as query builders, why do you need your own query builder layer in front?  Also if you think that separating business from data in case layers and tiers and technology need to be moved around in future is "architecture astronaut territory" then that is simply a reflection on the projects you have worked on yourself and nothing more.

    Tuesday, July 8, 2014 1:34 PM
  • User-1611549905 posted

    "All the way through to your view" is a straw man -- I'm not suggesting that, though I could see times when you may want to access your transaction manager in your controllers. You're also locking yourself out of a lot more than just lazy loading in terms of ORM -- as I said above, you're making a rod for your back if you ever need to implement cross-cutting concerns when saving or updating entities, or caching, or iterating through a very large data set that doesn't all fit in memory. There are also a whole raft of other problems, some of them quite subtle, that you're likely to come across too. Oren Eini, aka Ayende Rahien goes into this in more detail in his post, "The false myth of encapsulating data access in the DAL."

    The point I'm making is that true separation of concerns here is nowhere near as straightforward as you think. You're dealing with a complex set of abstractions, and since all abstractions are leaky, you're just stacking up problems for yourself.

    Wednesday, July 9, 2014 4:53 AM
  • User-760709272 posted

    None of your arguments seem to hold any particular weight.  In terms of cutting myself out of things....you haven't given any concrete examples of what I'm cutting myself out of that I haven't already addressed.  I don't see where caching comes into it either, and disconnecting your backend from your front end doesn't stop you dealing with large data sets too big to fit in memory.  It stops you returning them to the front end...but that's a good thing.  Again I'm having this nagging doubt that your like for this approach is one of laziness rather than anything else.  As long as you properly design things you shouldn't have any issues.  Maybe you don't fully get what I'm describing as the link you posted doesn't really have much relevance to what I'm saying...I'm talking about decoupling the repository clients from the underlying data access, not abstracting the data access.  When dealing with these issues it often comes down to accepting the lesser of the evils as it is rare that any architecture has no downsides at all.  Seeing as you dismiss off-hand any cons to your solution as "astronaut territory", and can only really find invented cons in other solutoins (that you may have got the wrong end of the stick over), maybe you should spend less time taking other people's articles out of context and spend more time programming in the real world :)

    Wednesday, July 9, 2014 5:27 AM
  • User-1611549905 posted

    In terms of cutting myself out of things....you haven't given any concrete examples of what I'm cutting myself out of that I haven't already addressed.

    You haven't adequately addressed lazy loading for starters. This is a very real problem of which you were quite dismissive: there are times when you will need to switch lazy loading on or off for the same query in different contexts depending on the size and scope of the data being returned, otherwise you'll end up crucifying performance either with the select n+1 problem or by grabbing too much unnecessary data. This is very often a decision that can only be made in your business layer rather than your repository, and if you're returning an IList from your repository rather than an IDbSet (and hence don't have access to Entity Framework's Include() method), you're not going to be able to do that.

    Nor have you addressed cross-cutting concerns: for example, when you need to trigger a refresh of some XML files when objects from any one of twenty or more model classes change and their state is saved to any one of a similar number of database tables. This was something I had to do once a few years back with NHibernate, and I ended up using an IInterceptor for it. Getting that past our implementation of the repository pattern was another whole level of fun.

    Maybe you don't fully get what I'm describing as the link you posted doesn't really have much relevance to what I'm saying.

    On the contrary, it's very relevant. The point Ayende is making is that your DAL is a much more complex abstraction than you think, and if you take that together with what Joel Spolsky said about every abstraction being leaky, the take-away is that the neat separation of concerns that you're advocating simply isn't going to be as neat as you think it is, and if you try to take advantage of the benefits that your separation of concerns claims to offer, you'll find that it simply doesn't work.

    I'm talking about decoupling the repository clients from the underlying data access, not abstracting the data access.

    I presume that means that you're talking about having different front-ends interfacing onto your business layer, rather than your business layer interfacing to different data sources? If so, that's not what you said in your previous reply, where you talked about moving from one type of data source to another. If not, then what exactly do you mean?

    In either case though, you're still going to run up against the problems that Ayende described in his post -- you're dealing with a complex and leaky abstraction, and things aren't going to be anywhere near as tidy as you think they are. You will sooner or later come across places where either your separation of concerns will get in the way, or else it will fail to offer the benefits that you expect it to offer because the abstractions are too leaky in ways that you didn't take into account. See Ayende's post again for some concrete examples.

    Again I'm having this nagging doubt that your like for this approach is one of laziness rather than anything else.

    No, it's one of experience. Experience of having inherited multiple codebases written with a tidy BLL/BOL/DAL architecture whose performance could only be measured in geologic eras. Experience of "best practices" that got in the way without providing any real benefit whatsoever. Experience of anaemic business "logic" layers that didn't have any logic in them whatsoever but just shunted data between one anaemic data model and another identical anaemic data model in a different namespace. Experience of seeing other frameworks in other languages (e.g. Python/Django, Ruby on Rails etc) make a much better job of architecting their solutions. Experience of spinning up a pet project of my own at least partly to experiment with what works best and what doesn't.

    In any case, don't be so dismissive of favouring simpler ("lazier") options. To quote Edsger W. Dijkstra, "Simplicity is prerequisite for reliability" -- more complex approaches carry an increased risk of introducing bugs, so any extra effort involved needs to be able to justify itself.

    Thursday, July 10, 2014 10:55 AM
  • User-760709272 posted

    You haven't adequately addressed lazy loading for starters.

    I have, I said it isn't really possible.  I also went on to say that if you need it your code or user interface isn't well designed.  If you really need it you can still get around the issue with subsequent calls to the repo.  That's all your ORM is doing under the covers anyway.  Not as easy as using an ORM that supports lazy loading, but I have admitted that from the start.  The number of times I've ever neded to use lazy loading is about zero, so having to throw a little more code around for the time that I ever do need to use it doesn't unduely concern me.  I'll certainly take the issue above having to couple my presentation layer to my database.

     

    Nor have you addressed cross-cutting concerns: for example, when you need to trigger a refresh of some XML files when objects from any one of twenty or more model classes change and their state is saved to any one of a similar number of database tables. This was something I had to do once a few years back with NHibernate, and I ended up using an IInterceptor for it. Getting that past our implementation of the repository pattern was another whole level of fun.

    I don't see how using a repo for this woul be too much of a hassle, but again it's not a situation I've ever had to address so I'm not going to give it a lot of thought.  I'm certainly not going to let one thing you had problems doing using NHibernate a few years ago make me couple my presentation layer to my database.

    On the contrary, it's very relevant. The point Ayende is making is that your DAL is a much more complex abstraction than you think

    And the point I've already made is that I'm not talking about abstracting the data layer, I'm simply saying it should not be tightly coupled to the other layers.  Who is fighting straw men now?

    In either case though, you're still going to run up against the problems that Ayende described in his post -- you're dealing with a complex and leaky abstraction

    See above.

    As I've already said, if you want to tightly couple your layers together due to the benefits it supplies then that's up to you.  If you want to dismiss the issues that will cause when layers need to move then that's up to you too.  If doing something simple like moving where you ToList means I get the benefits of any native ORM features (apart from lazy loading), and gives me a more flexible architecture that makes it less of a headache to move things around, then that's just what I'll do :)  Also, again as already said, we're discussing what makes something a repository and what a repository should do.  The argument about if you should use a repo in the first place is an entirely different thread.

     

     

    Thursday, July 10, 2014 11:49 AM
  • User-1611549905 posted

    There's one point where I'm not sure where you're coming from here:

    AidyF

    I'm not talking about abstracting the data layer, I'm simply saying it should not be tightly coupled to the other layers.

    As far as I understand it, these are, for all practical purposes considered here, two different ways of saying exactly the same thing. Or am I missing something here? If so, what, exactly, do you understand the difference to be? In particular, what aspects of the difference mean that Ayende's and Joel's analyses do not apply and why?

    Thursday, July 10, 2014 12:37 PM
  • User-1611549905 posted

    AidyF

    As I've already said, if you want to tightly couple your layers together due to the benefits it supplies then that's up to you.  If you want to dismiss the issues that will cause when layers need to move then that's up to you too.

    And as I've already said, my point is not that you shouldn't decouple your layers in the way you're suggesting, but that you can't. Yes, you may implement something that looks like a nice neat separation of concerns, but if you actually try to swap out one data source for another in the way you've suggested, you'll quickly find that you almost certainly haven't separated your concerns out as well as you think you have. You have to consider behaviour as well as just method signatures and interface implementations, and that is where it all gets messy.

    The fact of the matter is that unless the projects you are working on are all very simple, you will need to control lazy loading, access transactions, shape queries, handle cross-cutting concerns, batch updates and deletes, manage concurrency, and do a whole lot of other things like that in your business layer that require direct access to more advanced features of your ORM. Take query shaping for instance -- the most obvious (and simple) example here is filtering, paging and sorting data for display in a table. If you're insisting on putting ToList() in your repository, you're either going to end up with terrible performance (because you're returning masses and masses of data that you don't need) or else you're effectively moving your business logic into your repository, and you'll end up with an Anaemic Business Layer that doesn't do anything except shunt data from one anaemic domain model to another identical anaemic domain model -- and if you're doing that, then what's the point of having a separate business layer in the first place?

    So to answer the OP's question, the best place to put ToList() is in your business layer, if in fact you use ToList() at all. It shouldn't be automatic: it really needs to be treated on a case by case basis.

    As for moving the layers or swapping out your data source, I've cried "architecture astronaut" here because these are scenarios that I've seen repeated parrot fashion time and time again over the past twelve or so years that I've been working with .net, with little or no discussion as to whether (a) you're actually going to have to face them in the first place (you usually aren't), or (b) the "best practices" that you're following actually facilitate them in reality without introducing more problems than they solve (they usually don't). In actual fact, when you do need to scale out, it's never as simple in practice as shifting your presentation layer onto a separate server and sticking a service in between. You usually have to adopt other approaches such as caching, or sharding, or moving whole chunks of functionality into a separate application altogether, or switching from a traditional layered architecture to a CQRS/Event Sourcing one.

    Saturday, July 12, 2014 4:48 AM