Cyclomatic Complexity when I want to check the correct values of the arguments

Question

I have a parameter with 3 parameters and i want to ensure that data from parameters are correct, so I have something like that:

if param1 is null throw exception ArgumentNullException
if param1.Property < 0 throw exception
if para1.Property > 100 throw exception
if param2 is null throw exception
if param2.Property is null throw exception
if param2.Property is not > 0 throw exception

and so on.

In this case I have a cyclomatic complexity of 7, and it seems that between 6 and 10 it is hard to maintenace, so it is recommended 5 or less.

I was thinking to create a method that checks if each parameter is correct, but if I have 3 parameters and 3 methdos, the complexity is 4. So I only can use one loop, or one if more...

So my question is, I want to ensure that the data that I receive from parameters are correct and later, if all is correct, do the work, but I don't know how can I check my parameters and implement my code and keep a complexity below 6.

Thanks.

This might be better asked on Code Review than Stack Overflow (but check their rules first) — DavidG, Mar 17 '17 at 16:10
I suggest you to add code which you have now (with complexity 7) and move question to http://codereview.stackexchange.com for improvement — Sergey Berezovskiy, Mar 17 '17 at 16:14
_" 6 and 10 it is hard to maintenace,"_ - well [MS says 10](https://blogs.msdn.microsoft.com/zainnab/2011/05/17/code-metrics-cyclomatic-complexity/) and [nDepend say 15](http://www.ndepend.com/docs/code-metrics#CC), so 7 would seem to be ok. The easiest way to cut down CC is to split your method — , Mar 17 '17 at 16:16

Nathangrad · Accepted Answer · 2017-03-17T16:41:00.297

Personally, whenever I want to validate parameters, I evaluate exactly what types of validation is required (null checking, min / max values, date range etc). Once this is done, I create methods in a static utility class accepting parameters for validation to return a bool value indicating success.

Here's three methods which I'm using in a project of mine for validation:

public static bool NullCheck(params object[] parameters)
{
    foreach (object parameter in parameters)
        if (parameter == null)
            return false;
    return true;
}

public static bool MinCheck(int minimum, params int[] parameters)
{
    foreach (int parameter in parameters)
        if (parameter < minimum)
            return false;
    return true;
}

public static bool MaxCheck(int maximum, params int[] parameters)
{
    foreach (int parameter in parameters)
        if (parameter > maximum)
            return false;
    return true;
}

It's usage is as follows:

if (!NullCheck(obj1, obj2, obj3, obj4)) throw new ArgumentNullException("Invalid params");
else if (!MinCheck(5, int1, int2, int3, int4)) throw new Exception();
else if (!MaxCheck(8, int5, int6, int7, int8)) throw new Exception();

Edit

If the caller is required to know which parameter is invalid, rather than returning a bool value, it can return a Tuple<bool, object[]> where bool indicates the success / failure of the method and object[] contains any invalid objects.
Sample implementation included below:

public static Tuple<bool, object[]> NullCheck(params object[] parameters)
{
    var failures = new List<object>();
    foreach (object parameter in parameters)        
        if (parameter == null)
            failures.Add(parameter);
    if (failures.Count == 0)
        return new Tuple<bool, object[]>(true, null);
    else
        return new Tuple<bool, object[]>(false, failures.ToArray());
}

Each parameter needs to be processed and I think this is more of question about maintainability rather than performance. Having methods like these eliminates the requirement for `if... else...` or `object[] = parameters` `foreach parameter in..` with every method you want to have the parameters validated for. — Nathangrad, Mar 17 '17 at 16:57
Yes you are right, a single loop can do the job of multiple `if` so the net CC should be lower. Excellent! +1 — , Mar 17 '17 at 17:08
The general idea is good, but sometimes if I have a collection I use to check if the value of each correct, and use your solution makes me check all at the beggining of the method, so I have to iterate twice the collection, one for check if it is correct and other to process the items. But it is true that with code I reduce the if...else significally. — Álvaro García, Mar 17 '17 at 17:31
@ÁlvaroGarcía You could have a method accepting `params ICollection[]` as the parameters. This would enable you to loop through the `ICollection` to validate each record. This can easily be done in the same way as the sample methods previously provided :) — Nathangrad, Mar 17 '17 at 17:40

Cyclomatic Complexity when I want to check the correct values of the arguments

1 Answers1