views:

151

answers:

4

The function that needs to be tested is:

 int hire(Payroll * p) throw(out_of_range, logic_error)
 {

      // error if no holes left

      if (staffcount == MAX_EMPLOYEES)
                     throw (out_of_range("Hire: Too many employees"));

      // spin down holes looking for a hole.
      for (int i = 0; i < MAX_EMPLOYEES; ++i) {
          Payroll *current = staff[i].get(); // get the pointer

          if (current == 0) { // it is empty
              appay newpay(p); // convert  *Payrollto auto_ptr
              staff[i] =newpay;
              staffcount++; // one more staff
              return i;      // return index
          } else {
              // do nothing. Hole is filled
          }
      }
      // should never get here
      throw (logic_error("no holes, but count ok"));  }

I am able to test it by throwing an out_of_range error, but I can't think of any logic_error.

Here is my test in the main for out_of_range:

try {
    for (int i = 0; i<11; i++){
        hr.hire(new Payroll("Prog M. Er", 55757575));
        hr.showAllStaff(" after hires");
    }
} catch (out_of_range e) {
    cout << "Out of range error: " << e.what() << endl;
    cout << "DEBUG: carry on processing - line 177 was tested\n";
}

Any help on how to write a logic_error test for this function would be greatly appreciated! Thank you.

A

+1  A: 

You don't really need the optimization at the beginning. If you omit that then you only have one exception that you need to throw, you can't make logic errors and you don't have any problem testing it. Slower, yes, but if you're throwing an exception anyway, I doubt it makes any difference. It gives also a simpler function. Here's the code:

int hire(Payroll * p) throw (out_of_range)
{
    // spin down holes looking for a hole.
    for (int i = 0; i < MAX_EMPLOYEES; ++i)
    {
        Payroll *current = staff[i].get(); // get the pointer

        if (current == 0) { // it is empty
            appay newpay(p); // convert  *Payrollto auto_ptr
            staff[i] =newpay;
            staffcount++; // one more staff
            return i;      // return index
        } else {
            // do nothing. Hole is filled
        }
    }

    // error if no holes left
    throw (out_of_range("Hire: Too many employees"));
}
Mark Byers
A: 

There are two ways I can think of to hit that exception.

One would be to mutate MAX_EMPLOYEES after comparing to staff_count, but before you've finished executing the for loop. You'd have to have another thread do this, and hope it runs at the right time.

Two would be modify the hr.staff array without using the hire method. If you fill the staff array with MAX_EMPLOYEES Payroll objects and then call the hire method, then you hit that exception. You would probably want a friend class to do this, as I assume the staff array is private.

patros
+1  A: 

This is probably an area in which programming practices differ, but I would raise an assertion rather than throw an exception here.

Assertions are used to indicate "that which should never happen"; a programming error, possibly causing internal data corruption, or a severe violation of the assumptions under which the code was written.

Exceptions are generally used to indicate unexpected or unusual runtime errors (out of disk space, unexpected network errors, etc.).

If staffcount and staff are out of sync, that indicates a programming error and likely data corruption, and aborting the program (with a good error trace) may be preferable to continuing with corrupt data.

C has a built-in assert function, but alternatives are available, such as the lightweight Boost.Assert (which I use) and the Alexandrescu's and Torjo's very full-featured SMART_ASSERT library.

Josh Kelley
I would say it depends: for internal testing an assertion is nice, because you get the whole stack to understand the problem. When you put a client in front of your application, you definitely want something BETTER than a core dump!
Matthieu M.
+1 for the SMART_ASSERT library link, didn't know about it and the article is really enlightening (even got a bit of macro trickery!)
Matthieu M.
That's one reason to use something like Boost.Assert or SMART_ASSERT instead of the built-in assert function; you can keep the assert-style logic but use something more user-friendly than a core dump for release builds.
Josh Kelley
A: 

The question here is what you really need to test. I would say that what you need to test is whether the rest of your system will behave acceptably once the exception is thrown. Given that, you might temporarily replace this function with one that simply throws the exception, make sure it is handled correctly by whatever is supposed to catch it, then restore the correct function.

Joe Mabel