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

[Proposal] Add Guid-backed id which disallows Guid.Empty #84

Closed
hankovich opened this issue Oct 4, 2022 · 3 comments · Fixed by #117
Closed

[Proposal] Add Guid-backed id which disallows Guid.Empty #84

hankovich opened this issue Oct 4, 2022 · 3 comments · Fixed by #117

Comments

@hankovich
Copy link

We use guid-backed ids a lot, but since they may represent Empty (which it almost never a possible value), we have to do a lot of

if (id == TId.Empty)
{
    throw null!; // well, any stupid handling
}

checks. It looks like adding a new backing type (NotEmptyGuid) will be handy.

Probably we can go even deeper and introduce a new AllowEmpty parameter, because empty strings and zero ints/longs are also usually represent invalid identifiers.

@Rain0Ash
Copy link

Not possible.
Guid.Empty == default(Guid).
StrongId is struct type.
So, default(StrongId) where typeof(UNDERLYING) == typeof(Guid) - is also Guid.Empty.

@Varveyn
Copy link

Varveyn commented Oct 20, 2023

Might be covered in the scope of #75. Having the default constructor generation disabled, the developer could just define their own constructor that will disallow Guid.Empty (or string.Empty, or 0).

@andrewlock
Copy link
Owner

As Rain0Ash pointed out, this in nice theory, but doesn't really work for structs, because you can always do new MyId() using the default constructor.

Nevertheless, I'm currently working on a 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. So if you want to do something like throw in the constructor when provided Guid.Empty, you can 🙂

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.

4 participants