views:

83

answers:

4

Hi everyone,

I have a simple design(?) question.

I'm writing a simple program, that has a couple of functions that look like these.

float foo (float* m,size_t n){

   float result;
   //do some calculations, for example a sum 


   return result / n;
}

I have a couple of questions on this, with no intention of re opening some holy war.

Should I add a sanity check on the n ? If so, how should I let the caller know?

Returning -1 looks weird on floats;

float foo(float *m,size_t n){
     if (n == 0) return -1f

     ...
  }

My other option is an out parameter

float foo(float *m,size_t n, int *error){

        if (n==0){
           *error = 1;
            return 0f;
        }
       ...
}

update

This is kind of a toy program, just trying to practice some stuff. The question excedes that fact. Maybe I should rephrase to "how to handle errors without (OOP) exceptions".

Also considering testing n before doing the call, but don't like it as much.

Any thoughts? Thanks in advance.

+2  A: 

If -1 would not be returned by the function anyway, by all means return -1. But if passing n=0 won't break the function, then it isn't really needed. I assume that n is the size of the array m.

Handling errors is a matter of preference. OpenGL handles errors by returning an error code (-1 or otherwise) when a function fails. The error code is returned througha call to GetLastError() (or something like that). This seems like an ideal error handling solution.

Alexander Rafferty
yes, n is the number of elements in m.
Tom
+5  A: 

I guess your out parameter option is a good one. But I guess it would be better the other way. Use the out parameter to get the result and the return value to denote the status of the call. Like this

int foo(float *m, size_t n, float* result)
{
  if(someFailureCondition)
    return ERROR; // ERROR being an error integer
  // else
  // do some calculation
  // set your result
  return NO_ERROR; // NO_ERROR being an integer
}

Edit: The return value can be more verbose to denote the current state of the out parameter. See Jamesdlin's comment!

bdhar
Thanks, didn't consider that one. Seems more stylish.
Tom
I prefer this style too for checking for runtime errors, but you additionally should explicitly state (as part of the function's contract) what the state of the output parameters are on failure (e.g. are they in an undefined state, left untouched, or set to some specific value).
jamesdlin
+1  A: 

There are special floating point values you can use if you want - for example, if your floating-point implementation supports quiet NaNs (Not-a-Number) then you can use the NAN macro from math.h:

#include <math.h>
float foo(float *m,size_t n)
{
     if (n == 0) return NAN;

     ...
}
caf
Considered it, but isn't it the "do nothing" case?
Tom
I'm not sure what you mean. It's supposed to be similar to your `-1f` case, except that you test for it with `isnan()`, and it will propagate through subsequent floating-point computations.
caf
A: 

You should let callers know what your function's semantics are by clearly documenting your code.

What is the contract to your function? If callers are required to not pass 0 for n, then that needs to be explained, and the function should use assert to verify that those requirements are met. Logical errors should be detected early, and those failures should be as spectacular as possible.

Now, if you're writing code for a library that will be consumed by other developers and are concerned that people will compile with assert disabled, then it's reasonable to combine that with a softer failure mode that's always enabled:

if (n == 0)
{
    assert(0);
    return NAN; /* Or return some error code */
}
jamesdlin