3
\$\begingroup\$

I wrote a couple of extension methods for common guard statements I write a lot. These are extension methods for Object, and allow me to quickly guard against common scenarios.

I'm using .Net 3.5 (Unity3D) so sadly I don't have access to Code Contracts in this project.

My main concern with this code is there's a lot of duplication (primarily so that I can support custom messages for when it is suitable to do so). Is there a good way I can reduce this duplication and still support optional custom messages? I intend to add more common guard methods for the float, string and int classes too, but would rather have a good methodology in place

Any other comments on the code or tests are also welcome.

public static class ObjectExtensions
{
    public static void ThrowIfNull(this object value, string argumentName)
    {
        if (value == null) 
            throw new ArgumentNullException(argumentName);
    }

    public static void ThrowIfNotEqual(this object value, string argumentName, object expectedValue)
    {
        if (!value.Equals(expectedValue)) 
            throw new ArgumentException(argumentName, string.Format("Argument must not equal {0}", expectedValue));
    }

    public static void ThrowIfEqual(this object value, string argumentName, object expectedValue)
    {
        if (value.Equals(expectedValue)) 
            throw new ArgumentException(argumentName, string.Format("Argument must equal {0}", expectedValue));
    }

    public static void ThrowIfNull(this object value, string argumentName, string customMessage)
    {
        if (value == null) 
            throw new ArgumentNullException(argumentName, customMessage);
    }

    public static void ThrowIfNotEqual(this object value, string argumentName, object expectedValue, string customMessage)
    {
        if (!value.Equals(expectedValue)) 
            throw new ArgumentException(argumentName, customMessage);
    }

    public static void ThrowIfEqual(this object value, string argumentName, object expectedValue, string customMessage)
    {
        if (value.Equals(expectedValue)) 
            throw new ArgumentException(argumentName, customMessage);
    }
}

NUnit tests (should they be helpful):

[TestFixture()]
public class ObjectExtensionsTests
{
    [Test()]
    [ExpectedException(typeof(ArgumentNullException))]
    public void ThrowIfNullTest_NullInput_ThrowsException()
    {
        object subject = null;
        subject.ThrowIfNull("subject");
    }

    [Test()]
    public void ThrowIfNullTest_NonNullInput()
    {
        object subject = "Hello";
        subject.ThrowIfNull("subject");
    }

    [Test()]
    [ExpectedException(typeof(ArgumentException))]
    public void ThrowIfNotEqualTest_NotEqual_ThrowsException()
    {
        object a = 5f;
        object b = 10f;
        a.ThrowIfNotEqual("a", b);
    }

    [Test()]
    public void ThrowIfNotEqualTest_Equal()
    {
        object a = 5f;
        object b = 5f;
        a.ThrowIfNotEqual("a", b);
    }

    [Test()]
    [ExpectedException(typeof(ArgumentException))]
    public void ThrowIfEqualTest_Equal_ThrowsException()
    {
        object a = 5f;
        object b = 5f;
        a.ThrowIfEqual("a", b);
    }

    [Test()]
    public void ThrowIfEqualTest_NotEqual()
    {
        object a = 5f;
        object b = 10f;
        a.ThrowIfEqual("a", b);
    }

    [Test()]
    [ExpectedException(typeof(ArgumentException))]
    public void ThrowIfEqualTest_CustomMessage_Equal_ThrowsException()
    {
        object a = 5f;
        object b = 5f;
        a.ThrowIfEqual("a", b, "a is equal to b");
    }

    [Test()]
    public void ThrowIfEqualTest_CustomMessage_NotEqual()
    {
        object a = 5f;
        object b = 10f;
        a.ThrowIfEqual("a", b, "a is equal to b");
    }

    [Test()]
    [ExpectedException(typeof(ArgumentNullException))]
    public void ThrowIfNullTest_CustomMessage_NullInput_ThrowsException()
    {
        object subject = null;
        subject.ThrowIfNull("subject", "subject is null");
    }

    [Test()]
    public void ThrowIfNullTest_CustomMessage_NonNullInput()
    {
        object subject = "Hello";
        subject.ThrowIfNull("subject", "subject is null");
    }

    [Test()]
    [ExpectedException(typeof(ArgumentException))]
    public void ThrowIfNotEqualTest_CustomMessage_NotEqual_ThrowsException()
    {
        object a = 5f;
        object b = 10f;
        a.ThrowIfNotEqual("a", b, "a is not equal to b");
    }

    [Test()]
    public void ThrowIfNotEqualTest_CustomMessage_Equal()
    {
        object a = 5f;
        object b = 5f;
        a.ThrowIfNotEqual("a", b, "a is not equal to b");
    }
}
\$\endgroup\$
3
\$\begingroup\$

In my opinion, the extension methods are not adding a lot of value. You'd probably be better off without them.

Let's take a look at an example:

public void MyMethod(string value)
{
    if (value == null) throw new ArgumentNullException("value", "custom message");
}

vs

public void MyMethod(string value)
{
    ThrowIfNull(value, "value", "custom message");
}

In both cases you have about the same amount of information. The programmer needs to decide:

  • which method to call (ThrowIfNull) vs which exception to throw (ArgumentNullException)
  • which parameter to deal with ("value")
  • what the custom message looks like ("custom message")

The only real benefit to the extension method is the condition in the if block (== null), which they could theoretically stuff up and create a bug.

However, there are also some downsides to using the extension methods:

  • another programmer reading the code now has to deal with the cognitive load of learning what the extension methods are
  • when the exception is thrown, the debugger will break inside the extension method, not the method that caused the exception.
  • you've created a lot more work for yourself, for very little benefit.

Don't take my word for it. I read this stuff in a blog post once.

http://blog.ploeh.dk/2014/08/07/why-dry/

\$\endgroup\$
  • \$\begingroup\$ In this case the value is that I don't have to write out the throw again and don't have to provide a custom message unless the default does not make sense. So most cases it would be value.ThrowIfEqual("value",2); vs `if(value==2) throw new ArgumentException("value", "Argument must not equal 2");' but I do see your point about additional cognitive load (although I'd have hoped that these extension methods were named easily enough that a programmer reading it would understand their purpose) and also that the exception would be thrown in the wrong place. \$\endgroup\$ – Nick Udell Oct 20 '14 at 8:09
  • 2
    \$\begingroup\$ For reference for anybody reading this later, there is a [DebuggerHidden] attribute which will correctly show in a debugger that the exception was fired on the method calling line, not in the extension method. \$\endgroup\$ – Nick Udell Oct 20 '14 at 8:19
1
\$\begingroup\$

You using object for everything and hence no type checking. You shouldn't be able to compare oranges with apples, and the solution is simply to use generics. One more thing your code doesn't is convenience, what if I want to check if this value is equal to a value that a I can get from a method or from a different object without doing eager execution, and the solution is using lambdas.

 public static void ThrowIfNotEqual<T>(this T value, Func<T> expected)
    {
        if(!value.Equals(expected())){
            throw new ArgumentException ();
        }
    }

And this how I call it.

var st = "hi";
st.ThrowIfNotEqual(() => "hi" );

Say, I want to optimize it more, and only check for equality if the object is not equal to null

public static void ThrowIfNotEqual<T>(this T value, Func<T> expected)
{
  if(value ==null && !value.Equals(expected())){
        throw new ArgumentException ();
  }
}

Because of the short circuit here the function expected will not be called if value is null, which is not the case if you pass the object directly.

\$\endgroup\$
  • \$\begingroup\$ I'm confused on the lambda side here, isn't that executing the method anyway? What's the difference between ThrowIfNotEqual(foo, Bar()) (my approach) and ThrowIfNotEqual(foo, Bar) (your approach)? Doesn't your method then make it less convenient to evaluate against a variable? \$\endgroup\$ – Nick Udell Oct 18 '14 at 16:34
  • \$\begingroup\$ What if you want to call ThrowIfNoEqaul based on a condition? I can store its value in a higher order function and call it if necessary \$\endgroup\$ – Sleiman Jneidi Oct 18 '14 at 16:37
  • \$\begingroup\$ Could you edit your answer to include an example of this? \$\endgroup\$ – Nick Udell Oct 18 '14 at 16:42
  • \$\begingroup\$ @NickUdell I've edited in a way that keeps it relevant to the question \$\endgroup\$ – Sleiman Jneidi Oct 18 '14 at 16:48
  • \$\begingroup\$ Deleted my previous comment because I completely forgot about the NullReferenceException I'd get comparing null values. For your point on generics, does using generics here not prevent IEquatableimplementers being equated? \$\endgroup\$ – Nick Udell Oct 18 '14 at 17:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.