-1

I have a class with some members whose value has a finite range. For example

class Sphere
{
public:
    void setRadius(double radius)
    {
        m_radius = radius;
    }

private:
    double m_radius; // must >= 0.
};

I also have a dialog to input the radius. I can check the radius validation in the setRadius() method, or check in the dialog. Which way is better? This seems quite a common problem. What is the conventional way or best way? Thanks.

user1899020
  • 13,167
  • 21
  • 79
  • 154

5 Answers5

3

I assume the set method will be called based on user input. If so the conventional way is for the set method to throw an exception if exceptional value is provided. The exception should be propagated all the way to the UI

Alok Save
  • 202,538
  • 53
  • 430
  • 533
1

I can check the radius validation in the setRadius() method, or check in the dialog. Which way is better?

If the system is really small and GUI is relatively simple, you could check the input in dialog class. The other solution is to provide a 3rd class as input verification policy which checks user input.

Below is a very simple demostration code, could use template to set different policies for different GUI.

class CheckPolicy
{
public:
  CheckPolicy() {}

  virtual bool ValidInput(double f)
  {
    return f > 0;
  }
};

class GUI
{
public:
  GUI(){}
  void GetInput()
  {
      float f = 1.0f;
      if (policy_.ValidInput(f))
      {
        sphere_.setRadius(f);
      }
  }
private:
  CheckPolicy policy_;
  Sphere      sphere_;
};
billz
  • 44,644
  • 9
  • 83
  • 100
1

In OOP, you typically have the validation occur in the function and have the function throw an exception if the value is not within your accepted range, in your case.

However, if you're doing this for a class and aren't expected to know about throwing/catching exceptions yet, you can do one of two things:

  1. Have your function return an int, bool, etc. If the argument is within range, then your function returns some value indicating thus. Else, the function returns some value indicating the argument was not within the range.

Something like this:

bool setRadius(double radius)
{
    if(radius >= 0)
    {
        m_radius = radius;
        return true;
    }
    else return false;
}

Notice that you don't initiate/change the value stored in m_radius if the argument of "radius" is not within your range. There would be no point in changing the value of m_radius to an unacceptable value -- it's a waste of time.

  1. Check it in main() ("in the dialog"?)

If you want to know more about exceptions, you can refer to this page for a greater understanding of what an exception is: http://www.cplusplus.com/doc/tutorial/exceptions/

So, i++ and to conclude, the convention is to check it in the method and have the method throw an exception if the value is out of the accepted range. You may not have to if you're in a class and haven't covered exceptions yet though. Best of luck to you! :)

1

I'd suggest a different approach than the rest here. I'd just store the value there. Instead, looking into my crystal ball, when calling the Render(Image& target) function on the sphere that is where I would validate that all parameters fit. Also possible is a hybrid approach, where you validate that the radius is a non-negative float in the setter while you verify that the sphere is within the bound of the image when rendering.

There is a fundamental difference between the to approaches:

  • Not allowing the radius to become negative effectively means that a non-negative radius is a class invariant. Putting a validation of all class invariants into a utility function that you call via an RAII helper on entry and exit of all (mutating) memberfunctions is a good thing for debugging, as it immediately catches when the internal state (which is likely to be more complex than just a single scalar) become inconsistent in some way. For the sake of speed, I usually disable these checks for release binaries, so you don't even lose any performance even for extensive checks.
  • Allowing the radius to become negative but raising exceptions later turns this into a fault caused by the operation (e.g. rendering) that uses the value. In some cases, this makes the invalid input value indistinguishable from an out-of-memory during that operation, but it has the advantage that you only need to trap these errors in one place.
Ulrich Eckhardt
  • 16,572
  • 3
  • 28
  • 55
0

In my opinion, that is too much of object orientation. For less than two-three variables, I won't create a class having getter-setter and a private data - it doesn't make sense. It needs a lot of unnecessary code.

You better do the data-validation in the UI front (for example via DoDataExchange).

If you must devise a class to hold data and do data-validations, use templates!

Ajay
  • 18,086
  • 12
  • 59
  • 105
  • I am very interested in your method. I have lots of classes with only several members. Do you mean make those members public? Can you show me a very simple example for using templates to hold data and do data validations? Thanks a lot!! – user1899020 Jan 20 '13 at 05:24
  • First tell us (in your question), what you are using for UI. MFC? Win32? Qt? Show some code! – Ajay Jan 20 '13 at 05:28
  • I plan to use Qt and am still in the design stage. – user1899020 Jan 20 '13 at 05:38
  • 1
    Take a look at http://stackoverflow.com/q/148511/1968182 - I haven't browsed all of that, but the principle should be clear. – Ulrich Eckhardt Jan 20 '13 at 11:24
  • That solution mostly uses static assert, and dynamic assert, and is not viable solution for data validation. – Ajay Jan 20 '13 at 13:01