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

Implement missing Guid(string) constructor #64

Closed
MovGP0 opened this issue May 11, 2022 · 5 comments · Fixed by #117
Closed

Implement missing Guid(string) constructor #64

MovGP0 opened this issue May 11, 2022 · 5 comments · Fixed by #117

Comments

@MovGP0
Copy link

MovGP0 commented May 11, 2022

When implementing a type like

[StronglyTypedId(backingType: StronglyTypedIdBackingType.Guid)]
public partial struct CustomerId { }

Then instead of

var customerId = new CustomerId(new Guid("04004919-0a3a-47b5-89e0-92215ab9ff0f"))

I want to write

var customerId = new CustomerId("04004919-0a3a-47b5-89e0-92215ab9ff0f"). 
@andrewlock
Copy link
Owner

I can understand why you want it, but I'm not very inclined to add it, as otherwise why not allow it for ints, longs, all the other types. Where's the line? Why not also new CustomerId(byte[]) etc. Also, where is that string coming from? IMO you ideally should be using a converter to go directly from your source to the ID rather than through the string intermediate step

@MovGP0
Copy link
Author

MovGP0 commented Jun 13, 2022

why not allow it for ints, longs, all the other types.

those base types don't have a constructor of type T(string); they use parse/tryparse functions instead that we might want to implement instead.

Why not also new CustomerId(byte[]) etc.

Good idea.

Also, where is that string coming from?

I have the issue with migrating existing unit tests.

IMO you ideally should be using a converter to go directly from your source to the ID> rather than through the string intermediate step

Using the converter would be even more code in the unit tests then using the existing constructors.

@C0DK
Copy link

C0DK commented Aug 24, 2022

If you have to migrate a lot of unit tests, it might be a symptom of poorly written fixtures, that could need improvement.

But other than that, i do agree that it's nice to have the direct constructor to make the API of the Ids more userfriendly.

@MovGP0
Copy link
Author

MovGP0 commented Aug 24, 2022

If you have to migrate a lot of unit tests, it might be a symptom of poorly written fixtures, that could need improvement.

In my case it's a poorly written SQL database, where almost every table uses a GUID as the primary key.

@andrewlock
Copy link
Owner

For feature requests like this, I think I've found an answer in the big redesign of the library in this PR:

The main idea is to make the library much more maintainable while also giving people a mechanism to customise the generated IDs as much as they like. This will make it easy to make changes like this 🙂

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 a pull request may close this issue.

3 participants