4

Suppose I have a class like this

class A
{
    private int _x; //must be between [-10 10]
    private int _y; //must be between [10 20]
}

Using properties I have a couple of variants for the variables

public int X
{
    get { return _x; }
    set
    {
        if (!(value >= -10 && value <= 10))
            throw new Exception("Some error text");
        _x = value;
    }
}

or

public int X
{
    get { return _x; }
    set
    {
        if (!(value >= -10 && value <= 10))
            return;
        _x = value;
    }
}

(same for _y).
In first case I’m not sure I want to use exceptions (they are slow and have other known problems). In second I’m paying for not using them with the ambiguity of values (in some different way for _x and _y). Obviously I can use in my situation (if dealing with a list of objects) smth like that

public bool IsValid = true;
//...
public int X
{
    get { return _x; }
    set
    {
        if (!(value >= -10 && value <= 10))
        {
            IsValid = false;
            return;
        }
        _x = value;
    }
}
class AWrapper
{
    public List<A> AList {get; set;}
    public AWrapper(List<A> list)
    {
        AList = list.Where(x => x.IsValid == true).ToList();
    }

}

or some validator class or… I guess some other things. So I just want to form the criteria for myself – which technique is better and when…

Community
  • 1
  • 1
Alex Kovanev
  • 1,858
  • 1
  • 16
  • 29
  • 3
    Throw the exception. It's better to know ASAP that an operation fails rather than having to inspect some variable just to figure that out. If you're familiar with C at all, it's like having to choose between `errno` and exceptions. Wasn't always the nicest to use. – Jeff Mercado Oct 14 '11 at 10:06

6 Answers6

7

Lets get something straight to start with; Exceptions may be "slow", but thats exactly why you only use them in exceptional circumstances.

In your case, if a number falling outside a given range is Exceptional (ie, should never happen) then you're fine to throw an exception. If, however, this is something like user-input then the user filling in a value outside of an acceptable range is definately not exceptional! In this case what you want is validation that the value entered is within an acceptable range.

Another point on this solution:

set
{
    if (!(value >= -10 && value <= 10))
        return;
    _x = value;
}

This, IMO, is just as bad (if not worse). The reason being that if I set X=10, I expect that when I read X it has the value I just set.

Jamiec
  • 133,658
  • 13
  • 134
  • 193
2

Common approach is to raise InvalidArgumentException in such case. If you do not want to deal with exceptions you can introduce custom flag IsValid (as you already mentioned).

To persist a property range I would suggest introduce custom attribute for this purposes and mark properties by it like:

public bool IsValid { }

[ValueRange(Max = 10, Min = 5)]
public int X
{
    set
    {
       this.ValidateValueRange(this.X, value);
    }
}

private bool ValidateValueRange(...)
{
   // 1. Get property value (see link below regarding retrieving a property)
   // 2. Get ValueRange attribute values
   // 3. Update this.IsValid
   // 4. Return ...
}

And then implement single method to check whether passed in value is in range.

Useful links:

sll
  • 61,540
  • 22
  • 104
  • 156
2

I try to give some flow how to decide what to use here:

If you do not expect frequent "invalid" cases, definitively go for using exceptions.

set
{
    if (!(value >= -10 && value <= 10))
        throw new Exception("Some error text");
    _x = value;
}

Else if you need max performance and if it is acceptable, you can document your setter and state that all values outside the range are just ignored.

set
{
    if (value >= -10 && value <= 10)
        _x = value;
    // else optionally log something
}

If silently ignoring incorrect values is not an option, add logging or an IsValid flag like in your example. But beware you move the resposibility of detecting/checking for problems towards the caller. Exceptions are better as you must handle them otherwise you are directly punished.

The use of custom attributes presented in another answer is just another way of implementing the checking.

I hope these hints are helpful.

jdehaan
  • 19,700
  • 6
  • 57
  • 97
0

The standard pattern in .Net for checking for value validness or not is to create a method that checks the value first, let the user of the class call it and deal with the result and throw exception after that.

For example:

public bool IsValueValid(xxx)
{

}

public void SetValue(xxx)
{
    if(!this.IsValueValid())
    {
        throw Exception();
    }
}

Usually the class that has to store the value does not know what to do with an invalid value, it's the work of the caller to know how to deal with that value.

Ignacio Soler Garcia
  • 21,122
  • 31
  • 128
  • 207
0

Stick with

public int X
{
    get { return _x; }
    set
    {
        if (!(value >= -10 && value <= 10))
            throw new Exception("Some error text");
        _x = value;
    }
}
crypted
  • 10,118
  • 3
  • 39
  • 52
0

If you want to validate user input received from UI, then I would recommend you to go with Validator approach wherein you have the ability to inform user on what is expected.

It's a trade-off in other cases where you have decide the cost of not showing the exception and continue with any safe/default value or interrupt the flow with custom exceptions.

s_nair
  • 812
  • 4
  • 12