New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expose Top or Take #47
Comments
Hey @ovation22 , If I understood correctly, you want to expose What would be the benefit actually? Not writing the 0? |
It's a good point about the two being exclusive of one another. @ovation22 and I were discussing this Friday and my first thought was that it would be more intuitive to offer a .Take directly rather than having to "page" to do it. Conceptually to me at least, I don't think about paging as a way to get "Top X Rows/Records". Under the covers I know paging uses Skip/Take but it still seems like a bit of a hack to have to go through the Paging abstraction just to take advantage of this fact. It shouldn't be too hard for us to support Take or Paginate, and throw if both are used, right? |
Not a problem to implement. I was just thinking how to provide a consistent API. Not a huge fan of overlapping functionalities :) But, I have a feeling we misunderstood @ovation22. He might want to get top records regardless of the specification (for any spec). If this is a case, this can be done in your repository implementation.
PS. I just noticed |
I was specifically referencing using Top/Take in a Specification, but I appreciate the option with the IRepository. Giving it more thought over the weekend I was thinking along those lines same lines as you mention @fiseni if it would make sense to expose Take and Skip and deprecating Paginate. Makes sense to me. |
One of the key benefits of the Specification pattern IMO is that it helps prevent the need for repository implementations to have tons of different methods. If we can avoid adding more methods to repository by allowing fewer methods to do the same thing using a specification, I think we should do that. I also don't want to require devs to have to use our base repository just because they want to use specifications (as that will hurt adoption). So, I think your original idea works:
I'm still not sure I want to drop |
Sounds like we all agree. Add Take/Skip to specification and mark Paginate obsolete. |
I do share your opinion regarding the repositories. I didn't mean to provide the functionality through the repo, or enforce it in any way. It's just an example how users can implement such thing in their code base, in their repo (cos that's the only way you can get such general Top functionality). I'm free for the next 30 mins, I'll do the changes now. |
This is implemented. If no one has additional comments, we may close the issue. |
Ok so all I need to do if it looks good is publish a new point release, yes? |
Yes, I think so. Other than that, tests should be refactored. It's in my backlog too, if I find time in the next period, I'll try to go through that. |
Looking to select example: 4 items from my specification. I've resorted to using Query.Paginate(0, 4).
@ardalis suggested raising an Issue here to see if it might be a trivial item to implement.
The text was updated successfully, but these errors were encountered: