andrewlock / StronglyTypedId

A Rosyln-powered generator for strongly-typed IDs
MIT License
1.52k stars 80 forks source link

Implement missing Guid(string) constructor #64

Closed MovGP0 closed 9 months ago

MovGP0 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 9 months ago

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 🙂