3
\$\begingroup\$

This is a ( single file ) library I wrote as an helper to check arguments validity.Here below a sample usage:

public class Demo
    {
        /// <summary>
        /// Classical not null verification
        /// </summary>
        /// <param name="arg1"></param>
        public void DoSomething(string arg1)
        {
            Contract.Expect(() => arg1).IsNotNull();
        }
        /// <summary>
        /// Asserting an argument greather than another
        /// </summary>
        /// <param name="min"></param>
        /// <param name="max"></param>
        public void DoSomething(int min,int max)
        {
            Contract.Expect(() => min)
                .IsLessThan(int.MaxValue)
                .Meet(a => a <= max);
        }
        /// <summary>
        /// Validate an integer argument in a certain range
        /// </summary>
        /// <param name="arg1"></param>
        public void DoSomething(int arg1)
        {
            Contract.Expect(() => arg1).IsGreatherThan(0)
                                       .IsLessThan(100);
                ;

        }
        /// <summary>
        /// Validate an array in length and asserts first element should be zero
        /// </summary>
        /// <param name="array"></param>
        public void DoSomething(int[] array)
        {
            Contract.Expect(() => array).Meet(a => a.Length > 0 && a.First() == 0);
        }

    }

Exception message are auto generated based on the argument names, so we don't need magic strings at all. What about it ? Improvement and suggestions? Entire code is hosted here.

EDIT just to clarify the pourpose of the library: I'm trying to avoid code such as:

if (stringParameter == null)
    throw new ArgumentNullException("stringParameter");

in which we need to write a string to specify the parameter name. This is not refactory friendly, neither DRY since you write twice "stringParameter" instead of:

Contract.Expect(() => stringParameter).IsNotNull();

That is once, and refactor friendly ( at least in my opinion ;) ).

\$\endgroup\$
  • 1
    \$\begingroup\$ I'm curious, why don't you use .NET 4 Code Contracts? \$\endgroup\$ – Quentin Pradet Mar 15 '12 at 14:11
  • \$\begingroup\$ @Cygal they don't help to avoid typing the name of the argument as a string \$\endgroup\$ – Felice Pollano Mar 15 '12 at 14:28
  • \$\begingroup\$ @Felice Pollano. please elaborate on "they don't help avoid typing the name of the argument as a string". I have not used the contracts enough. \$\endgroup\$ – Leonid Mar 18 '12 at 2:54
  • \$\begingroup\$ @Leonid if I have to add into the emssage the name of the argumet, I hace to type it as a string, if I refactor, I Have to change etc etc. \$\endgroup\$ – Felice Pollano Mar 18 '12 at 9:43
  • \$\begingroup\$ This looks like a good generic implementation of a concept I have been messing around with for a little while. I think my goal is the same as yours, to have an internal validator that checks for common errors, checking arguments passed into a public-facing API. Like you, I do not like having the repetition of 'throw new ArgumentNullException("stringParameter")' and 'throw new ArgumentException(String.Format("'{0}' is not a valid xxx.","parametername"))' at the top of all my non-private methods. If you don't object, I would like to use your Contract class in a real project. \$\endgroup\$ – Mike Feb 29 '16 at 22:10
2
\$\begingroup\$

Felice, can you expand on what you are looking for? Are you trying to infer constraints to be applied dynamically? I looked at your link to the whole code base at bitbucket.

First of all, nice work. Looks like you spent a good deal of time whipping that up.

On the surface, given your demo and not knowing what exactly your intended use case is, it appears 1) that you are trying to make constraints more visually fluent (which I appreciate) and 2) it seems "possibly" unneeded.

Once you further expand on your use case, I'm sure it will make more sense and I can revise my statements. However, for now let me explain item #2. 1) You can add the fluency you require by refactoring your code base into helper methods. 2) I would not really recommend throwing exceptions from what I can see. Given the code sample below (from your Contracts.cs) you are throwing an exception on something that should merely be a boolean check. As such you are altering the behavior of what you normally come to expect out of a programming language.

public IContract<T> IsEqual<TT>(TT of) where TT : T,IEquatable<TT>
{
    TT actual = (TT)accessor();
    if (!actual.Equals(of))
    {
        throw new ArgumentOutOfRangeException(Name, "Must be equal to" + of.ToString());
    }
    return this;
}

3) I would recommend watching out for "re-inventing the language". Sometimes it is hard to identify, but the general smell is that you try to recreate basic functionality in the toolset you already use, instead of just using what you have. This can result in double/triple programming. Let's use the demo example from above (focusing on the snippet I grabbed). You have built several methods to provided the ".IsNotNull()" method for the Contract and are then encapsulating that (to make it more readable and fluent) into a method that just takes the parameter.

/// <summary>
/// Classical not null verification
/// </summary>
/// <param name="arg1"></param>
public void DoSomething(string arg1)
{
    Contract.Expect(() => arg1).IsNotNull();
}

instead of just:

if (arg1.IsNullOrEmpty())

or

if (arg1.IsNullOrWhiteSpace())

Which are already built into the .net framework.

I hope some/all of this helps. If you would, please further explain your use case. As for now, my recommendation would be to refactor your utility into static helper methods and not throw exceptions. For the error message part, just create helper method(s) to get the error message needed. So 1 method does the needed check and if it fails...and as a programmer you want to get the error message, you can choose to do that in a separate call.

\$\endgroup\$
  • \$\begingroup\$ thank for taking the time. I'm looking for a whole review of my code, and opinion if the library is felt as useful. The goal of the library is doing argument validation without using "magic strrings" This mean: throwing a proper exception if the argument does not meets some pre-condition. \$\endgroup\$ – Felice Pollano Mar 15 '12 at 13:08
  • \$\begingroup\$ Thanks for the clarification Felice. I have not extensively utilized .net contracts so code review wise, I would defer to a more experienced person. However, after reviewing .net contract and re-reviewing your library, I think it is a good idea you have that you are auto-replying with the correct error message instead of deferring to the user to hand type each time. \$\endgroup\$ – Randy Mar 15 '12 at 15:49
  • \$\begingroup\$ Thank yopu for your opinion, I've already +1 your review. I wait some other answer too before to mark as a reply. \$\endgroup\$ – Felice Pollano Mar 15 '12 at 16:01

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.