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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
grofit opened this issue Feb 23, 2022 · 4 comments

Comments

@grofit
Copy link
Contributor

grofit commented Feb 23, 2022

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
Copy link
Collaborator

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

Do you have any examples of this use?

@grofit
Copy link
Contributor Author

grofit commented Feb 23, 2022

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
Copy link
Collaborator

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
Copy link
Contributor Author

grofit commented Mar 5, 2022

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.

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

No branches or pull requests

2 participants