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

integrate and refactor ThrowIfNull- fixes #5055 #5068

Closed
wants to merge 1 commit into from

Conversation

r-52
Copy link

@r-52 r-52 commented Sep 24, 2022

This PR introduces a backward compatible way to use ThrowIfNull to check if the given argument is null and
throw an exception or simply return the value of the argument.

This is a draft PR to get initial feedback. It implements a backward compatible attribute that's used on .NET 5 and lower. The checks are an example how it could be used in the Logger.cs class.

An idea that came: maybe the Guard class could be extended to also integrate more checks like ThrowIfNullOrEmpty for e.g. strings?

R=@snakefoot?
Refs: #5055

@welcome
Copy link

welcome bot commented Sep 24, 2022

Thanks for opening this pull request!
We will try to review this soon! Please note that pull requests with unit tests are earlier accepted 👼

This PR introduces a backward compatible way to use `ThrowIfNull` to
check if the given argument is null and
throw an exception or simply return the value of the argument.
@r-52 r-52 force-pushed the feature/r-apply-throw-if-null branch from 73db656 to 8f5b9ff Compare September 24, 2022 17:53
@sonarcloud
Copy link

sonarcloud bot commented Sep 24, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

50.0% 50.0% Coverage
0.0% 0.0% Duplication

@snakefoot
Copy link
Contributor

Think it looks good. Just need a single unit-test that verifies that ArgumentNullException thrown includes the expected input-parameter-name.

@snakefoot
Copy link
Contributor

Doesn't sound like a bad idea to also include ThrowIfNullOrEmpty for string-values. But maybe for a different pull-request?

@eli-entelis
Copy link

@r-52 , is there a reason the file is called guard? shouldn't it be something like NullExecptionHandler?

@snakefoot
Copy link
Contributor

snakefoot commented May 2, 2023

Superseeded by #5211 - Thank you for providing this initial code-suggestion, it helped getting the ball rolling.

@snakefoot snakefoot closed this May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants