Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use IDbConnection instead of DbConnection. #2

Closed
wants to merge 1 commit into from

Conversation

moonheart
Copy link

I think we should depends on interfaces rather than concrete classes. How do you think馃槃

@codeapologist
Copy link
Owner

My initial implementation actually used IDbConnection, but I later discovered that Dapper's internal implementation is coupled with DbConnection object. So I made a change to the accept DbConnection to ensure working functionality. You can see the release note here: https://www.nuget.org/packages/DataAbstractions.Dapper/1.0.2

@moonheart
Copy link
Author

I see. The IDbConnection does not have an OpenAsync method which 'SqlMapper.Async' depends on.
As these issues descripted https://github.com/dotnet/corefx/issues/3480#issuecomment-159533059, DapperLib/Dapper#757
Thanks for your detailed explanation, I'll close this PR now.

@codeapologist
Copy link
Owner

I had a change of heart on this. Dapper is handling OpenAsync appropriately when provided a DbConnection vs IDbConnection, and it is the developer's responsibility to use the connection object correctly. So with that being said, I have updated the constructor signature to accept IDbConnection instead of DbConnection in version 3.1.0. This isn't a breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants