Dapper.Contrib icon indicating copy to clipboard operation
Dapper.Contrib copied to clipboard

Dapper.Contrib inconsistencies and future development

Open alexwiese opened this issue 7 years ago • 0 comments

I've just started using Dapper.Contrib in a project and hit a few bugs. I've already submitted a couple of PRs for these. In saying that it looks like Dapper.Contrib needs a bit of love.

Here are just a few issues I've hit:

  • There is a lot of potential for issue with the property caching (ie. cache methods expose the instances of their underlying storage List<> allowing callers to modify the cache - DapperLib/Dapper#418).
  • Formatter/provider is not always used for formatting ColumnName and ColumnNameEquals statements, some places use hard coded formatting strings which may only work 100% for certain database engines/connection types
  • I've noticed inconsistencies between sync and async implementations; some bugs have been fixed in the sync method but not the async version. It would be good to get the common logic unified where possible.
  • Code/file structuring (13 classes/all code is in two files), not sure if this was done for a reason. IMO it would be nice to split it out into separate files and use partial classes a la Dapper proper. I'd hope it would make code maintenance and PR review a lot nicer

I could spend quite a bit of time tidying this code up and addressing some of the above points. I am happy to have a crack at this, however before I do that and submit a PR that may or may not be considered/merged I'd ask:

  • Is now a good time to do these changes? I know that a lot of the StackExchange OSS projects are going through the "Great Migration" to .NET Standard/Core
  • Is Dapper.Contrib still active in development? I notice a lot of outstanding PRs and Issues being left unresolved

alexwiese avatar Jun 19 '18 02:06 alexwiese