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

CollectionArg methods disregard optsFn for null checks #161

Closed
DennisInSky opened this issue Jul 7, 2021 · 6 comments · Fixed by #162
Closed

CollectionArg methods disregard optsFn for null checks #161

DennisInSky opened this issue Jul 7, 2021 · 6 comments · Fixed by #162
Milestone

Comments

@DennisInSky
Copy link

DennisInSky commented Jul 7, 2021

CollectionArg methods disregard optsFn for null checks
https://github.com/danielwertheim/Ensure.That/blob/master/src/projects/EnsureThat/Enforcers/CollectionArg.cs#L19
unlike it is done for StringArg
https://github.com/danielwertheim/Ensure.That/blob/master/src/projects/EnsureThat/Enforcers/StringArg.cs#L17

@DennisInSky DennisInSky changed the title CollectionArg methods disregards optsFn for null checks CollectionArg methods disregard optsFn for null checks Jul 7, 2021
danielwertheim added a commit that referenced this issue Oct 11, 2021
@danielwertheim danielwertheim mentioned this issue Oct 11, 2021
@danielwertheim danielwertheim added this to the v11 milestone Oct 11, 2021
@danielwertheim
Copy link
Owner

@DennisInSky what use case do you use the options for? The reason for me asking is that I’m leaning against removing it. Why? Because the lib should only be for argument validation (guard clause) and not for generating custom business exceptions etc. And if so, what in options is needed?

@DennisInSky
Copy link
Author

@danielwertheim I am using it for for guard clauses, but in some cases I want them to throw custom exceptions which I can handle and return error messages in a more user friendly way.

danielwertheim added a commit that referenced this issue Oct 18, 2021
* Fixes #159, Only supports netstandard2.0 and netstandard2.1

* Fixes #161

* Changes EnsureOptions to be readonly

* Change so that Ensure.That returns the Param

* NuGet upgrades

* Fixes benchmark

* Docs
@danielwertheim
Copy link
Owner

@DennisInSky how would you feel about a "Scope" construct instead? See #165 for a sample.

@DennisInSky
Copy link
Author

Well, I haven't dived deeply into the code, but will the scope affect all the clauses in the nested calls? I would definitely not want it. Your intent is to avoid using this library for throwing custom exceptions, but what is the reason of using it at all if I have already verified conditions using some other means which allow me throwing custom exceptions? I do not think one would like to write double checks.

@danielwertheim
Copy link
Owner

but what is the reason of using it at all if I have already verified conditions using some other means which allow me throwing custom exceptions?

What do you mean by "if I have already verified conditions"?

As long as you are in the scope that would provide scoped data for the exception factory. Which, yes would make life more cumbersome, as you probably would encapsulate your custom "rules" in a method e.g. EnsureFooIsValid(foo).

@ndrwrbgs
Copy link
Contributor

link to #165

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