Stop Asserting Arguments by Name

Asserting arguments is essentially a healthy practice. What I don’t like, is the fact that since day one ArgumentNullException used a string for argument name, and now with .NET 4.5 almost knocking on the door, there’s still only string option. 

 if (x == null)
    throw new ArgumentNullException("Value was null", "x");

Code Contracts introduced a while ago, has an implementation for Requires precondition that is good, yet still has the same issue. 

Contract.Requires<ArgumentNullException>( x != null, "x" );

Many other frameworks still rely on string as a source of the argument name for assertions these frameworks make (Sitecore as a such that I have recently used). 

Sitecore.Diagnostics.Assert.ArgumentNotNull(args, "args");

Why not to add an expression support and be done with it? This way, when you refactor code and “accidentally” rename your argument, assertion message is not going to lie. I’ve being using this code with assertions created by 3rd party:

    public static string Get_Name(Expression<Func<object>> func)
    {
      return (func.Body as MemberExpression).Member.Name;
    }

And no worries about re-naming my arguments.

14 Comments

  • I've created my own contract library, which uses an expression parameter. For example: Expect.NotNull(() => argName). There are other methods to check for NotNullOrEmpty, Range, DefaultValue, user defined conditions, etc. I find it really useful and more robust than the code examples that you have given (that is, I agree with you).

  • As much as I'd love not to have to type in the string every time, I'd wouldn't do this. And for performance reasons. Several reasons:

    - Cant be done currently without good performance hit.
    - Assertions are not something user should see. Therefore, it's not critically important that correct string is displayed.

    I ran a quick test of method you pasted. For 1,000,000 iterations, results are as follows:
    - string name = Get_Name( () => varA ); takes 15.635s
    - string name = "varA"; 0.01s

    You may say you won't have that many assertions, but it's not necessarily true. One service call may result in 4-5 method calls, which have 2-3 asserts, or even more. And also we wouldn't stop here, but implement more non-critical stuff using expressions.

    Don't get me wrong, I like this idea very much. Namely, I've been looking for a way to do safe null dereferencing and Expressions are the way to go. I stole of solution and improved it to use Expressions instead of Reflection, and it is faster. But at this point I'm not ready to trade performance for comfort.

    I'd really love to be able to do this :)
    obj.Deref(x => x.PropA.PropB.PropcC.PropD)

  • @NikolaR: Erh, we agree that this code is only called when there is an exception right?

  • Random thought - Why not have a library/utility function, something like (off top of my head)

    ThrowIfNullHelper( variableToTest, () => GetName( () => variableToTest ) )

    We'd only evaluate GetName() IF and Only if variableToTest is null

    Performance hit is minimal?

  • @NikolaR,
    You can actually achieve obj.Deref(x => x.PropA.PropB.PropcC.PropD) already. Take your expression body as a string, and with Regex take the part after the last dot :)
    Again, if performance speed is your concern, it won't be acceptable, but if it is not - solution is right there.

  • @ND,
    Precisely the idea :)

  • Sean- Actually all you need is JetBrains Resharper. It fills it in as well as renames the string argument name. Life is short, use Resharper and get done quicker. :)

  • @icecold_2,

    If life would be that easy :) I am working with a vendor team, they all have R# and no one is utilizing it. So?... Also, when you Rename with R#, you have to explicitly select to rename comments and strings. I don't do that, as it's not always working the way you expect it.

  • @Sean - In resharper version 6 (maybe also in the prev version) you do not have to explicitly select rename comments and strings. It just does it without searching comments and strings. Very nice :)

    If they don't want to use Resharper when they already have it then there is more keystrokes in the day and it is from their choice.

  • @icecold_2,
    I hear you, yet it doesn't solve the problem of utilizing strings :)

  • @Sean- I understand the need to get rid of magical strings. I *hate* having string values, the conclusion I came to awhile ago (before resharper) was performance related as NikolaR commented about.

    I think until there is a way the compiler can do it, using expression trees/reflection is not the way to go about being lazy (performance wise). If the apps you're working on aren't worried about performance then you've taken the right step, until you do care about that performance. Maybe y'all have taken an approach that has some kind of performance optimization and if so I'd like to see the numbers.

    Thanks for the post

  • @icecold_2,
    Thank you for the comments. It is interesting to know what others are doing about it. Keep it coming :)

  • Please don't use underscores in method names - it violates the framework design guidelines (and looks ugly :))

  • @Norei the wise,

    Here I'd have to strongly disagree with you. Design guidelines are not set in stone, and should not be interpreted as the one and only way to implement things. Now, underscores in method names - on contrary, myself and teams in the past have found that names with underscores provide better readability of long method names. Not to mention tests.

Comments have been disabled for this content.