1

I have a static class ArgumentHelper which can check for example: ArgumentHelper.NotNull(instance, "instance"). But it works for some standard built-in .net framework classes, if I implement my own classes, I cannot put checking in ArgumentHelper, because this class will be too big. So I was thinking, and I found a solution: I will make a nested static class in every new made custom class:

public class MyClass
{
    private int x;

    public MyClass (int x)
    {   
        if (x < 0) throw new ArgumentOutOfRangeException (...);

        this.x = x;
    }

    public static class Arg
    {
       public static void NotZero (MyClass mc)
       {
           if (mc.x == 0) throw ....
       }
    }
}

So now:

public void myMethod (MyClass mc)
{
    MyClass.Arg.NotZero (mc); // will throw excweption if x == 0

    do some stuff
}

Is it a good idea, or you have different approach?

DLeh
  • 23,806
  • 16
  • 84
  • 128
apocalypse
  • 5,764
  • 9
  • 47
  • 95
  • "if I implement my own classes, I cannot put checking in ArgumentHelper because this class will be too big" - why? If you're checking common things (nullity, argument out of range) then I expect you'll find the same conditions are used within many classes - why would you want to duplicate them? – Jon Skeet Dec 02 '14 at 15:08
  • 1
    I assume that you want to implement [`Debug.Assert`](http://msdn.microsoft.com/en-us/library/system.diagnostics.debug.assert%28v=vs.110%29.aspx): http://stackoverflow.com/questions/129120/when-should-i-use-debug-assert – Tim Schmelter Dec 02 '14 at 15:09
  • In my custom classes i dont check common things. Like i shown in example, i was checking the x is == 0. Maybe it is not good exmaple, but for more complicated scenario, if i want to check 3 variables, for example Point cannot be 0,0,0, it will produce 3 lines of code. And i want just Point.Arg.NotZero (p) – apocalypse Dec 02 '14 at 15:10

3 Answers3

1

There are a number of ways to do this, but I would say this way is unnecessarily complicated. Instead of creating classes all over the place you can create a single static class that has the logic for checking different parameters. For example:

 //Common methods for validating method parameters
 public static class Ensure
 {
      public static void NotNull(object o, string paramName)
      {
          if (null == o)
              throw new ArgumentNullException(paramName);
      }

      public static void NotZero(Point p, string paramName){ /*logic*/ }

      public static void GreaterThanZero(int i, string paramName)
      {
          if (i <= 0)
               throw new ArgumentException("Must be greater than zero", paramName);

      }
 }

Then in your class:

 public class MyCustomClass
 {
      public void SomeMethod(object o, int i, Point p)
      {
            //Guards
            Ensure.NotNull(o, "o");
            Ensure.GreaterThanZero(i, "i");
            Ensure.NotZero(p, "p");
      }
 }

Also, Microsoft Research also has a similar project called "Code Contracts". You can read more about it here: http://visualstudiomagazine.com/articles/2010/06/23/code-contracts.aspx

Update

In an effort to reduce the number of lines, you could do something like this:

public static class Ensure
{
    public static void NotNull(params object[] objects)
    {
       if (objects.Any(x => null == x)
          throw new ArgumentNullException();
    }
}

Then in you method:

public void example(object o, int i, Point p)
{
    Ensure.NotNull(o, i, p);
}
Philip Pittle
  • 11,821
  • 8
  • 59
  • 123
  • I have something like this implemented. But like you see, in SomeMethod you have ensure 3x for objecto o. So if you have 50 methods like that, you haver 150 extra lines. So Im doing methods likeEnsure.CHeckStateOfCustomClass. But after this, Ensure class I growing up. But even this is not problem. Problem is when i need to use custom class from different assembly. Ensure does not know about this assembly and cannot reference to it. – apocalypse Dec 02 '14 at 17:02
  • You could put the Ensure class in a seperate utility assembly that has no dependencies and can therefor be referenced by all of your project assemblies. As per having lots of lines of Ensure classes, I'll post a small update to make it a little better. The other option is to use something like PostSharp's NotNull attribute. – Philip Pittle Dec 02 '14 at 19:26
1

I prefer using better naming and a pass-through interface for this.

For example, you could have this class:

public static class ThrowOn
{
    public static T Null<T>(T val, string name) where T : object
    {
        if (val == null) throw new ArgumentNullException(name);
        return val;
    }
}

and use it in this way:

private MyObject _mine;

public SomeConstructor(MyObject mine)
{
    _mine = ThrowOn.Null(mine, "mine");
}

Which makes it perfectly clear that an exception will be thrown. I use generics to ensure that it will be type strong and you can't assign the result of the test to the wrong type.

I like this because it keeps your constructor nice and neat, factors the code out and it does exactly what it says on the box.

You might also name the class ThrowIf if you like the way that reads.

This could just as well be done in extension methods:

public static class Extensions
{
    public static T ThrowOnNull<T>(this T val, string name) where T : object
    {
        if (val == null) throw new ArgumentNullException(name);
        return val;
    }
}

which in use would look like this:

private MyObject _mine;

public SomeConstructor(MyObject mine)
{
    _mine = mine.ThrowOnNull("mine");
}

One argument I've heard against this approach is that the top of the stack frame is not the source of the exception, but I have more confidence in my customer's ability to read a stack crawl.

plinth
  • 48,267
  • 11
  • 78
  • 120
1

This is drastically different to my other answer, so I'm creating a new one. Another option is to use AOP via a framework like PostSharp. The caveat of this approach is AOP generally requires modifying the compiler to add custom compile steps or being coupled with Dependency Injection to work. Generally this is fine, but it can be considered a limitation depending on your environment.

PostSharp has a tutorial on this very issue that let's you decorate your method parameters with attributes that will enforce the "contracts":

public void Foo( [NotNull] string bar ) {}

PostSharp will rewrite this to (post compile time)

public void Foo(string bar ) 
{
    if (null == bar)
       throw new ArgumentNullExpcetion(bar, "bar");
}

You can read more about how this works and what's supported here: Validating parameters, fields and properties with PostSharp 3

Philip Pittle
  • 11,821
  • 8
  • 59
  • 123