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

Add Guard fro SmartEnum Out of Range #528

Open
MarkLFT opened this issue May 30, 2024 · 2 comments · May be fixed by #533
Open

Add Guard fro SmartEnum Out of Range #528

MarkLFT opened this issue May 30, 2024 · 2 comments · May be fixed by #533

Comments

@MarkLFT
Copy link

MarkLFT commented May 30, 2024

Plase add a guard for Smart Enums Out of Range, just like a regular enum.

Please refer to original discussion at ardalis/GuardClauses#346 (comment)

@Asafima
Copy link

Asafima commented Jun 19, 2024

Hey, I have read the original discussion.
As I see it, there are a few options for implementing this:

  1. Create a Guard folder with Guard class and interfaces, same as in GuardClauses repo, so that API consumers use it as: Guard.Against.SmartEnumOutOfRange(someValue)

  2. Create another static method in the SmartEnum abstract class - in that case the usage would be: TestEnum.ThrowIfOutOfRange(TValue value)

  3. Use GuardClauses nuget package as a dependency - least preferred, probably a bad idea and a major overhead for this small use case.

In any case, the method should check all enum options and find a matching one - it's actually IsDefined.
What do you think @ardalis ?

@ardalis
Copy link
Owner

ardalis commented Jun 19, 2024

My issue with option 1 is that I'm sure many projects already use both Ardalis.SmartEnum and Ardalis.GuardClauses, so the potential for naming conflicts is a problem. Let's say we add a Guard class, would it live in the namespace Ardalis.SmartEnum.Guard (the folder you suggest), and would it use the same extension method approach? If so, what happens if a file includes a using statement for Ardalis.GuardClauses? Now you have to fully qualify the extension method which is ugly.

Regarding option 3, I do think that most projects that use SmartEnum quite likely also use GuardClauses, but I wouldn't want to force any consumer to do so. So I agree, not ideal.

That leaves your option 2 of just emulating what Microsoft did with their Exception types and add ThrowIfSomeCondition methods to the Enum itself. I don't care for that syntax much myself as it doesn't provide the same clarify of Guard.Against (or even Assert) but at least there's some precedent for it.

A 4th option would be to create a new package on the SmartEnum side, like Ardalis.SmartEnum.GuardClauses which would reference Ardalis.GuardClauses and SmartEnum. This is clean from a dependency and namespace standpoint and lets us reuse the same Guard.Against pattern/usage, but at the cost of adding another package to maintain and depend upon (and a package that will probably only have a single file in it, at that).

Open to more comments/ideas/discussion.

@Asafima Asafima linked a pull request Jul 15, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants