search

tmsmith / Dapper-Extensions

Dapper Extensions is a small library that complements Dapper by adding basic CRUD operations (Get, Insert, Update, Delete) for your POCOs. For more advanced querying scenarios, Dapper Extensions provides a predicate system. The goal of this library is to keep your POCOs pure by not requiring any attributes or base class inheritance.
1.79k stars 585 forks source link

Is there a technical reason why the generics need to be constrained by class/reference types? #294

Open grofit opened 2 years ago

grofit commented 2 years ago

I just noticed when trying this out on a library that the generics NEED to be reference types, but is there any technical reason why it cant support struct/value types too?

valfrid-ly commented 2 years ago

Hi @grofit , we didn't implemented any support for structs.

Do you have any examples of this use?

grofit commented 2 years ago

Hey, not exactly but other orms do not enforce the generics are reference types so it was immediately noticable when I dropped in this one and all the generics required extra constraints.

While I imagine most people would be using classes, I do not find it super crazy that people may be representing their data as value types.

I thought originally maybe the issues were down to lack of nulls for value types but couldn't see any major issues when I looked in the source around that.

Given that both value and reference types for the most part are the same in terms of reflection based interactions, it just felt like it was a constraint that may not be needed, as ideally I wouldn't want the cascading generic constraints etc at repository level.

What specifically do you see as needing adding to the source to support structs? As from a brief look it seemed like you could pretty much just remove the generic constraints and most of the existing code base would work without much changing.

valfrid-ly commented 2 years ago

Hi @grofit ,

I really need help with this repository. I'm not the owner, as you may noticed.

Feel free please to update the code and submit a PR.

The most important rule I follow to approve the PR is that the unit tests need to be running OK and new code has to be covered.

I'm still working with coverage for some existing code.

grofit commented 2 years ago

I am sorry but I don't have loads of free time to put into other open source projects, as I have plenty of my own to work with and this was just a test run for the library for a scenario.

This all being said I did a quick fork and removed all references to where T : class and nothing blew up and all UNIT tests passed (didn't run integration as I wasn't entirely sure of what docker stuff needed running). I have put it up as a PR but as you mention ideally you want to verify all the tests pass so there is no regression, so feel free to run the branch on your env and if everything passes then even though there are no explicit struct tests nothing has regressed and you are no longer forcing consumers to use classes explicitly.