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

EnsureArg.IsNotNull without class requirement #140

Closed
cjvanwyk3 opened this issue May 8, 2020 · 7 comments · Fixed by #142
Closed

EnsureArg.IsNotNull without class requirement #140

cjvanwyk3 opened this issue May 8, 2020 · 7 comments · Fixed by #142
Labels

Comments

@cjvanwyk3
Copy link

Would it be possible to remove the class requirement on EnsureArg.IsNotNull? Maybe something like this:
public static T IsNotNull(T value, string paramName = null)
{
var type = typeof(T);
if ((type.IsClass || IsNullable(type)) && value == null)
{
throw new ArgumentNullException(paramName, "Value may not be null.");
}

        return value;
    }

    private static bool IsNullable(Type type) => Nullable.GetUnderlyingType(type) != null;
@danielwertheim
Copy link
Owner

Is this the case you want to support? E.g. a nullable int.

[Theory]
[InlineData(null)]
public void ShouldThrow_WhenNull(int? i) =>
    ShouldThrow<ArgumentNullException>(
        ExceptionMessages.Common_IsNotNull_Failed,
        () => Ensure.Any.IsNotNull(i, ParamName),
        () => EnsureArg.IsNotNull(i, ParamName),
        () => Ensure.That(i, ParamName).IsNotNull());

[Theory]
[InlineData(1)]
public void ShouldNotThrow_WhenNull(int? i) =>
    ShouldNotThrow(
        () => Ensure.Any.IsNotNull(i, ParamName),
        () => EnsureArg.IsNotNull(i, ParamName),
        () => Ensure.That(i, ParamName).IsNotNull());

image

@ndrwrbgs
Copy link
Contributor

ndrwrbgs commented May 9, 2020

It's worth noting that a struct cannot be null in C#. It can override .Equals to return true when compared to null, but it itself cannot be null - so I'd be cautious about the API exposed if any changes are needed for this.

int? a = null; is Equal to null, but is not actually null.

More actionable feedback -- since Nullable<T:struct> is a common structure, I would add support for that specifically (and maintain the existing API naming since == null is colloquially special-cased for that) but if the need is for a struct other than Nullable to be compared, I'd redirect towards the IComparable implementations.

@danielwertheim
Copy link
Owner

The code above are tests against existing code.

@ndrwrbgs
Copy link
Contributor

ndrwrbgs commented May 9, 2020

I apologize for my silliness thanks Daniel for the patience!
@cjvanwyk3 are you still blocked on this?

@cjvanwyk3
Copy link
Author

cjvanwyk3 commented May 11, 2020

What I'm ultimately after is being able to use EnsureArg on a generic that does not have a class requirement. For example

// This will not work.
        public void GenericMethod<T>(T parameter)
        {
            EnsureArg.IsNotNull(parameter);
        }

@ndrwrbgs
Copy link
Contributor

So sorry to have dropped this!

@cjvanwyk3 could you refactor your code to match more like what we have here under AnyArg?

[NotNull]
[ContractAnnotation("value:null => halt")]
public T IsNotNull<T>([NoEnumeration, ValidatedNotNull] T value, [InvokerParameterName] string paramName = null, OptsFn optsFn = null) where T : class
{
if (value == null)
throw Ensure.ExceptionFactory.ArgumentNullException(ExceptionMessages.Common_IsNotNull_Failed, paramName, optsFn);
return value;
}
[NotNull]
[ContractAnnotation("value:null => halt")]
public T? IsNotNull<T>([ValidatedNotNull] T? value, [InvokerParameterName] string paramName = null, OptsFn optsFn = null) where T : struct
{
if (value == null)
throw Ensure.ExceptionFactory.ArgumentNullException(ExceptionMessages.Common_IsNotNull_Failed, paramName, optsFn);
return value;
}

The thing is, we don't have an overload that checks != null in the case that '== null' is impossible. I'm trying to find a way that it makes sense in the general (I 100% relate to your use case), but am struggling to generalize it to everyone.

One work around that's always available as you want things that aren't in the base library (although thank you for asking since it helps improve coverage ♥) is that the Ensure.That() syntax returns a Param<> object you can write any extension method imaginable against. So even if a particular case doesn't make it into the code base, it can still be supported for your style!

@cjvanwyk3
Copy link
Author

@danielwertheim Thank you so much! The EnsureArg.HasValue(item); works great for us!

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 a pull request may close this issue.

3 participants