Added extension methods for IDbTransaction

[smcenlly]

I've added extension methods for IDbTransaction, makes the syntax more fluent when dealing with transactions.

[NickCraver]

While the commit is much appreciated - I'm not sure this is a common case, and it add's a lot of mostly-duplicate code to support such a case. To be honest: I'm not sure including this is worth the tradeoff in maintenance costs it introduces.

[smcenlly]

No problem. I'm happy to close the request if you're sure you don't want to include.

[NickCraver]

I don't think it's worth the cost, but I'm not so arrogant as to think I'm aware of all use cases and percentages in the world either :) Let's give it a chance and see if any others chime in with thoughts.

[mgravell]

This is actually something I thought about adding a while back; what held me back is - as you probably noticed - the annoying number of overloads you need to add to support this. If you are writing transaction based code, I totally get that this makes things more obvious and elegant than tran.Connection.Execute(sql, args, transaction: tran); (etc).

Given that the code exists, I'm not particularly against taking it.

[mgravell]

(note: I typo'd in my last comment, reversing the intent; my bad; tl;dr: I'm ok-ish with this)

[thomashjorslev]

I would welcome this addition. I recently discovered dapper and really like it. So far I have been using it to extend a large existing system, which uses explicit transactions. I found that for calls with many parameters, it is easy to forget the transaction argument, since it comes after the arguments.

[NickCraver]

Here's my fear: it doubles the surface area (and almost line count) for all future method additions as well. I'm don't think that is a good thing.

What if we instead put this in Dapper.Transactions (an additional project/nuget) that contained and maintained all of these convenience overloads? I'm certainly not against having them, just the weight it adds to the core codebase is something we can't really go back on.

Obviously this has the downside of "future methods may be forgotten" with respect to a transaction extension...but I think that's really true either way and not much less true in another file. It's also very easy to fix and deploy.

[mgravell]

I'd be good with the Dapper.Transactions (separate assembly) idea. If we
set it up via project.json, it is very little project overhead.
On 1 Jan 2016 1:11 p.m., "Nick Craver" notifications@github.com wrote:

Here's my fear: it doubles the surface area (and almost line count) for
all future method additions as well. I'm don't think that is a good
thing.

What if we instead put this in Dapper.Transactions (an additional
project/nuget) that contained and maintained all of these convenience
overloads? I'm certainly not against having them, just the weight it adds
to the core codebase is something we can't really go back on.

—
Reply to this email directly or view it on GitHub
#429 (comment)
.

[NickCraver]

Okay I'll be honest, I don't know what I was smoking in the earlier comment. How bout we just put these in a SqlMapper.Extensions.Transactions.cs (or a better name) partial and call it a day? I suppose there's really no need for a separate assembly and NuGet (user) overhead for approximately the same maintenance cost.

We could also add a unit test that crawls for methods and checks if we're missing any (which can have exceptions where there shouldn't be a corresponding overload for some reason).

[mgravell]

Although, it occurs: this is already missing 50% of the overloads... *Async. That's' a lot of additional code to add :(

[NickCraver]

Yeah...you just talked me out of supporting this. I'm not sure of a good middle-ground that doesn't involve that much duplication.

[smcenlly]

Closing pull request.