Skip to content
This repository has been archived by the owner on Oct 11, 2023. It is now read-only.

Improvements to throwIf methods #39

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pdimitratos
Copy link
Contributor

to improve tracing and increase flexibility of usage

Copy link
Member

@danieljurek danieljurek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍
Makes sense that ThrowIf.Null is useful outside of constructors.

Some small commentary for consideration.

=> o ?? throw (calledFromMember == ".ctor"
? (Exception)new ArgumentNullException(
paramName,
$"Null argument {paramName} passed to {calledFromMember}in file {calledFromFile} at line {sourceLineNumber}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The call stack will already have the calling method name, file, and line number info. Not sure if it's valuable to keep in here.

Also, if you do keep it, put a space between {calledFromMember}in...

$"{paramName} found to be null or whitespace in method {calledFromMember} in file {calledFromFile} at line {sourceLineNumber}",
paramName
)
: new NullReferenceException(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicking: If you're checking for whitespace/empty string you might want to throw an ArgumentException when the value is an empty or whitespace string.

[CallerLineNumber]int sourceLineNumber = -1
) where TParam : class
=> o ?? throw (calledFromMember == ".ctor"
? (Exception)new ArgumentNullException(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One weird corner case would be if the constructor was throwing because of null when checking something that's not an argument:

private object resultFromSomeMethod;
public MyClass(string param) 
{ 
  this.resultFromSomeMethod = SomeMethod(param);
  ThrowIf.Null(this.resultFromSomeMethod); 
}

Not sure if/how we should handle that scenario outside of using code analysis at build time.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants