Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

throw exceptions or not when check the parameters of a method? [closed]

Tags:

c#

exception

I like to check that all the parameters of the method have the correct information before doing something with them. Something like this:

public method(MyType param1)
{
try
{
    if(param1 == null)
    {
        throw new ArgumentNullException("Error1");
    }
    if(param1.Property1 == null)
    {
        throw new ArgumentNullException("Error2");
    }

    if(param1.Property1.Amount <= 0)
    {
        throw new ArgumentException("Error3");
    }

    ...


    //Do what I need with the parameter
}
catch { throw; }
}

However, in this post someone comments that it is not a good idea to throw an exception as normal flow, but I am not sure if this is the case or not, because if I have to check the parameters and also there are exceptions like ArgumentNullException and ArgumentException that it seems that it can be thrown when there are some problem with parameters, it makes me to wondering if really it is a bad way to do, the example that I comment.

Another reason that another user gives it is that an exception consume 4000-8000 cycles of CPU. Well, the objective in my case it is to know if there are some error with the parameters and really if the application works as expected, the exception will never be thrown, so in practice, if the application has no bugs then the performance doesn't decrease.

So in summary, I would like to know how is the best way to handle the check of the parameters before continuing with the process.

Thanks.

like image 758
Álvaro García Avatar asked Oct 18 '22 08:10

Álvaro García


2 Answers

You should definitely throw exceptions when a required value is null or missing. One of the things I like to do to clean up this type of situation is use a method to check if the Type is valid, and use Exception annotations when my code might throw an exception.

The down side is that, if used correctly the Validate(...) method gets called twice. I like the Validate(...) approach because it allows a change to find the error prior throwing the exception, because any class can call Validate(...)

class MyClass
{
    /// <summary>
    /// DoSomething
    /// </summary>
    /// <exception cref="InvalidOperationException">InvalidOperationException</exception>
    public void DoSomething(MyType myType)
    {
        string errorMessage;
        if (!Validate(myType, out errorMessage))
            throw new InvalidOperationException(string.Format("Argument myType is not valid: {0}", errorMessage));

        // Finish
    }
    /// <summary>
    /// IsValid
    /// </summary>
    /// <exception cref="ArgumentNullException">ArgumentNullException</exception>
    public static bool Validate(MyType myType, out string errorMessage)
    {
        errorMessage = null;

        if (myType == null)
            throw new ArgumentNullException("myType");           
        if (string.IsNullOrEmpty(myType.Property1))
            errorMessage = "Property1 is required";
        if (string.IsNullOrEmpty(myType.Property2))
            errorMessage = "Property2 is required";

        return errorMessage == null;
    }
}

class MyType
{
    public string Property1 { get; set; }

    public string Property2 { get; set; }
}
like image 133
geoffb-csharpguy Avatar answered Oct 21 '22 04:10

geoffb-csharpguy


Throwing exceptions after argument validation is a good approach. This way you explicitly let the developer using your method know that he is calling it incorrectly, which enables him to easily fix this bug.

I would definitely get rid of the try...catch part, though. It is absolutely redundant and makes the code more complex than it needs to be (and slightly confusing).

like image 31
Kapol Avatar answered Oct 21 '22 03:10

Kapol