I am wondering if what i am going to do is good or bad thing. I have that class:
public class Element : IElement
{
public float? Max { get; private set; }
public float? Min { get; private set; }
public float? Average { get; private set; }
public bool HasValue { get; private set; }
public void SetRange(float? min, float? max)
{
if (min >= max)
{
throw new WrongElementValueException("Min must be greater than max!");
}
else if (min < 0f || max < 0f)
{
throw new WrongElementValueException("Min/max must be greater than 0!");
}
else if (min > 100f || max > 100f)
{
throw new WrongElementValueException("Min/max must be lesser than 0!");
}
else
{
Min = min;
Max = max;
Average = (min + max)/2f;
HasValue = true;
}
}
}
The user will set the values using SetRange() method. But he has some constraints like Min must be bigger than Max, and neither of them should be bigger than 100 or lesser than 0.
Should I use those exceptions in this place? Or is there any better method to handle wrong user input? I hope my question isn't to general.
else if (min < 0f || max < 0f)
throw new WrongElementValueException("Min/max must be greater than 0!");
else if (min > 100f || max > 100f)
throw new WrongElementValueException("Min/max must be lesser than 0!");
I'd note that there is already ArgumentOutOfRangeException that's defined for precisely this sort of case.
if (min >= max)
throw new WrongElementValueException("Min must be greater than max!");
This should definitely be an ArgumentException, but if WrongElementValueException inherits from ArgumentException, then that's fine.
Your general approach is sound. I'd consider going further:
HasValue = true;
Why allow for the class to ever not have a value. Consider if you add:
public Element(float min, float max)
{
SetRange(min, max);
}
Now you can never have an instance without its values set, and can get rid of HasValue entirely.
Note though that I changed this from float? to float. You might well be advised to do that throughout the class. Otherwise if you have a need for cases where Min and Max are null, (and therefore don't want to get rid of HasValue) you need to catch that case in SetRange:
public void SetRange(float? min, float? max)
{
if (min < 0f || max < 0f || min > 100f || max > 100f)
throw new ArgumentOutOfRangeException();
if (min >= max)
throw new WrongElementValueException("Min must be greater than max!");
Min = min;
Max = max;
if(min.HasValue && max.HasValue)
{
Average = (min + max)/2f;
HasValue = true;
}
else
{
Average = null;
HasValue = false;
}
}
(I'd also generally favour double and double? over float and float? unless you've a strong reason otherwise).
Incidentally, we generally use "exception handling" to talk about how code that's using this code deals with the fact that your code threw an exception, with just what the best thing to do is depending on the context of that code rather than this code.
Collected from the Internet
Please contact [email protected] to delete if infringement.
Comments