iammukeshm/CleanArchitecture.WebApi

Feature request

binson143 opened this issue · 1 comments

binson143 commented

Describe alternatives you've considered
https://github.com/iammukeshm/CleanArchitecture.WebApi/blob/master/Application/Interfaces/IDateTimeService.cs.
This code is a good candidate to become an extension method

https://github.com/iammukeshm/CleanArchitecture.WebApi/blob/master/Application/Interfaces/IGenericRepositoryAsync.cs
There is a precedent to split this repository into the read-only repo and write-only repo (CQRS).

https://github.com/iammukeshm/CleanArchitecture.WebApi/blob/master/Application/Interfaces/IEmailService.cs
This can be part of the infrastructure code. moving this code into a common lib would be nice.

https://github.com/iammukeshm/CleanArchitecture.WebApi/blob/master/Application/Behaviours/ValidationBehaviour.cs
Throwing exception vs returning error from the business layer is subject to debate, but returning an error from the business layer seems more meaningful ( Honestly, this is a personal belief)
https://softwareengineering.stackexchange.com/questions/405038/result-object-vs-throwing-exceptions

https://github.com/iammukeshm/CleanArchitecture.WebApi/blob/master/Application/Wrappers/PagedResponse.cs
https://github.com/iammukeshm/CleanArchitecture.WebApi/blob/master/Application/Wrappers/Response.cs
The above code files are eligible to move into common lib.

https://github.com/iammukeshm/CleanArchitecture.WebApi/tree/master/Domain/Common
Context of a DDD approach, This can be moved to Sharable code space. ( SharedKernal/ common lib)

https://github.com/iammukeshm/CleanArchitecture.WebApi/blob/master/Domain/Common/BaseEntity.cs
Nice to have a generic version of BaseEntity to support different Id type (int,guid,string etc).

https://github.com/iammukeshm/CleanArchitecture.WebApi/blob/master/Domain/Common/AuditableBaseEntity.cs
Seems good but can be great by adding additional details like Ip, browser,

https://github.com/iammukeshm/CleanArchitecture.WebApi/tree/master/Domain/Settings
Not sure this is part of Domain, Seems good candidates for Infrastructure.

All the above comments are just personal thoughts. This project seems promising and appreciable. Good Job Man!

iammukeshm commented

Hi, Thanks a lot for the feedback :D Here is my take. My Opinion :P

  1. In theory, this was placed as interface/implementation to accommodate various DateTime logics during test cases. It would not always be DateTime.Now right? Test cases can be run for multiple time factors with this approach.

  2. I believe Repo Pattern creates an abstraction over the DataAccess Tech. 2 Years ahead and if EFCore is no longer the best ORM, it should be pretty simple to integrate/switch to a better ORM with less coding.

  3. Again, IEmailService is just an interface. There are n ways to send out emails like direct SMTP, mail relays, SendGrid, or basically any other provider. The core aim of the interface is to have different implementation at the infrastructure level. As of now, only SMTP is implemented. But it is very easy to have a Sendgrid Implementation too.

  4. Behavior pattern is something that i came across while working with mediatR library. Found this usage in several Microsoft Projects too. Retuning EXC from the BL may be good, but to have centralized control, I felt it better to be in the APP layer. So that different implementations won't hurt the message formats. Also, I had globalization concerns in my mind regarding error messages.

  5. Yes, Technically they are already in the Common Library. With correspondence to the app setup, Application/Core acts mostly as the common library for the Enterprise/Application. I wouldn't have a different DLL just to accommodate these response classes because the response is something that is very specific to a business. Making it a nugget package / separate project may not be re-usable always. So, don't you think it is better to maintain response classes specific to an application?

  6. Same answer as above.

  7. Yep, that's a cool idea. Might be overkill at this stage of the project. I Will put that to my list.

  8. Again, Settings is something common and core to the project. I wouldn't put them to the infrastructure with this kind of arch setup.