henkmollema/Dommel

Methods now require passing IDbConnection or SqlBuilder

TroySchmidt opened this issue · 5 comments

Resolving things like the table now require IDbConnection or ISqlBuilder so it can get access to the QuoteIdenitifier. Maybe this could be reworked to be an extension off IDbConnection if that is going to be required.

What value does an extension method add? They're advanced methods and only clutter Intellisense when using IDbConnection.

The implementation of adding the requirement of IDbConnection or ISqlBuilder for resolvers exploded the implementation of using the Resolvers. Perhaps a better solution is to provide a mechanism to provide an internal static ISqlBuilder akin to the way Resolvers are set and it access and use what is provided there. This was a huge breaking change and has made migrating to version 2 very painful.

I'm not sure how that would work. For the resolvers to work with the new QuoteIdentifier option, they have to know the database type being used. You can't use a static for this.

Make a new method in DommelMapper that is something like DommelMapper.SetSqlBuilder and that sets the static property for Resolvers.SqlBuilder then add new methods that if you don't pass that it uses the Resolvers.SqlBuilder by default. And by default that just returns the column name as is (which is the old behavior) and thereby wasn't as breaking of a change.

Adding a static field or new methods without ISqlBuilder adds lots of complexity, caching issues and possibly undesired behavior. I don't think the issue is really big, it's easy to use a static ISqlBuidler yourself when consuming the resolvers. Since you've already dealt with the breaking changes, I don't think there's much left to do here.