urfnet / URF.Core

Unit of Work & Repositories Framework - .NET Core, NET Standard, Entity Framework Core. 100% extensible & lightweight.
https://github.com/urfnet
MIT License
309 stars 62 forks source link

Extending/Customizing ITrackableRepository<TEntity> and IService<TEntity> for Team & Project Edge Cases #27

Closed rdhainaut closed 6 years ago

rdhainaut commented 6 years ago

First of all, thank you for your work (and for the new URF.Core)

I have noticed a difference between URF and URF.Core. A lot of synchronous methods (for repository, service and unit of work) are gone. Of course, the asynchronous methods will have a positive impact on the performance But often some piece of code need to run synchrone way. In that case, add an await operator for each instruction has a "negative" impact on the clarity of code (for example: in each test, you need to add an await operator for each method call).

I think this kind of choice should be do at the application level (and not at the framework level). So ideally, the framework should implement synchronous and asynchronous methods. What are your opinion about this ?

tonysneed commented 6 years ago

In URF.Core we are trying to create a pit of success for devs. I/O-bound operations should almost always be async. However, if you want to call them synchronously, for example in a unit test, then you can simply attach .Result to the call (rather than awaiting it). This is the approach taken by HttpClient, which only has an async API. So there is precedent for having only async methods.

That's the reasoning behind this design decision. Ordinarily I would agree with having both sync and async, and leaving it up to the dev to choose. But I've seen firsthand what many devs actually do when given the choice and observed them falling more often into the pit of despair rather than the pit of success. So the design choice is basically to nudge them in the right direction.

rdhainaut commented 6 years ago

Thanks to have take your time to write an answer. I understand better. The "pit of success" is an interesting design concept (it avoids many try/error trial).

lelong37 commented 6 years ago

@rdhainaut this was actually a good question, which also pushed us to do a PR with examples on how to extend IRepository, ITrackableRepsoitory, and IService so that you and other teams can extend URF for your project & teams specific use cases. We even took the liberty to add a synchronous method for you in the examples. The example illustrates best practices and patterns on how to achieve your use case by using two URF extensibility strategies. Whether one chooses to do this in the Repository layer or Service layer, we'll leave that decision with teams using URF. However, for you specific use case, if you were to add synchronous methods for CRUD, we would recommend that this implementation resides on the Repository layer since it is database (CRUD) concern with no business logic. We recommend that teams put any business logic in the Service layer and anything w/ database concern in the Repository layer.

Extending ITrackableRepository<TEntity>, we added a (example) synchronous Find() method, scope: application-wide at the Repository layer, across all ITrackableRepository<TEntity>'s.

Extending IService<TEntity>, we added a (example) synchronous Single() (example) method, scope: application-wide at the Service layer, across all Services e.g. ICustomerService, etc.

*For the examples, we've suffixed extensibility classes with X e.g. IRepositoryX.cs