3 ms·

Your example is wrong. Functions parameters should be guaranteed to be error-free in the first place. If you decided that a NULL string was an error condition

Your example is wrong. Functions parameters should be guaranteed to be error-free in the first place. If you decided that a NULL string was an error condition, you should test for that before calling printf. You should test return values, not parameters.

cperciva 16y ago
You're disagreeing with the wrong person here.
- loup-vaillant 16y ago
  I disagreed with your "extreme example", on the grounds that checking arguments instead of return values is a bad idea.
  - cperciva 16y ago
    Yes, I know. I also disagree with philwelch's suggestion of checking arguments -- that's why I presented that extreme example.
    - loup-vaillant 16y ago
      OK, let's pretend I was tired. I was disagreeing with the wrong comment…
philwelch 16y ago
If you have to check your return values yet you can't check your arguments, how do you compose functions? foo(bar(x)) requires foo to check its argument, since bar(x) could return an error code and foo() needs to know how to deal with it. You don't have to check all possible instances of spurious arguments--you should be allowed to specify some preconditions--but you definitely have to check for error codes! Example: Suppose we're writing a basic filesystem. We have the following primitive functions: unsigned long getino(char pathname): For a given designated device and path, get the inode number of the designated file. Return 0 (there is no inode number 0) in case of error. MINODE iget(unsigned long ino): For a given designated inode number, return a pointer to a data structure in memory containing the INODE data; allocated if necessary. Return NULL in case of error. You can either test your return values from these functions, in which case you will do this: unsigned long myIno = getino(path); //error check myIno MINODE * myMINODE = iget(myIno); //error check myMINODE or put some argument checking in iget() and do this: MINODE * myMINODE = iget(getino(path)); //error check myMINODE How much argument checking has to go into iget()? Exactly this much: if (!ino) return NULL It seemed like a neat idea at the time, and considering I called that particular composition of functions a dozen times in that project, there was a lot less repeating myself in teaching my functions to understand error codes.
- loup-vaillant 16y ago
  In general, I prefer not to have to check for arguments. In your case, the right™ way to do it is probably monads: they help you separate the error checking from the "normal" path. But that's C code, so I have to agree with you.
- cperciva 16y ago
  unsigned long getino(char * pathname): For a given designated device and path, get the inode number of the designated file. Return 0 (there is no inode number 0) in case of error. Completely off-topic, but there is a bug in this prototype: Inode numbers have type ino_t, not type unsigned long. (Ok, two bugs, if you count the fact that there can theoretically be an inode number 0. And three bugs if you count the fact that pathname should be declared as a (const char *). But the ino_t vs. unsigned long bug is really bad if you care about portability.)
  - philwelch 16y ago
    You're right on all counts, and if I were writing a real filesystem rather than a class assignment I would have taken these issues more seriously. (edited to add paragraph:) Nothing I've said in this discussion should be taken as a strongly held opinion about anything--I'm rather shy about sharing these thoughts in the first place, but considered this particular example illustrative of a possibility I take seriously. You do not want to see the other horrors that particular professor of mine has perpetrated, incidentally.

cperciva 16y ago

You're disagreeing with the wrong person here.

loup-vaillant 16y ago
I disagreed with your "extreme example", on the grounds that checking arguments instead of return values is a bad idea.
- cperciva 16y ago
  Yes, I know. I also disagree with philwelch's suggestion of checking arguments -- that's why I presented that extreme example.
  - loup-vaillant 16y ago
    OK, let's pretend I was tired. I was disagreeing with the wrong comment…

philwelch 16y ago

If you have to check your return values yet you can't check your arguments, how do you compose functions? foo(bar(x)) requires foo to check its argument, since bar(x) could return an error code and foo() needs to know how to deal with it. You don't have to check all possible instances of spurious arguments--you should be allowed to specify some preconditions--but you definitely have to check for error codes! Example: Suppose we're writing a basic filesystem. We have the following primitive functions: unsigned long getino(char pathname): For a given designated device and path, get the inode number of the designated file. Return 0 (there is no inode number 0) in case of error. MINODE iget(unsigned long ino): For a given designated inode number, return a pointer to a data structure in memory containing the INODE data; allocated if necessary. Return NULL in case of error. You can either test your return values from these functions, in which case you will do this: unsigned long myIno = getino(path); //error check myIno MINODE * myMINODE = iget(myIno); //error check myMINODE or put some argument checking in iget() and do this: MINODE * myMINODE = iget(getino(path)); //error check myMINODE How much argument checking has to go into iget()? Exactly this much: if (!ino) return NULL It seemed like a neat idea at the time, and considering I called that particular composition of functions a dozen times in that project, there was a lot less repeating myself in teaching my functions to understand error codes.

loup-vaillant 16y ago
In general, I prefer not to have to check for arguments. In your case, the right™ way to do it is probably monads: they help you separate the error checking from the "normal" path. But that's C code, so I have to agree with you.
cperciva 16y ago
unsigned long getino(char * pathname): For a given designated device and path, get the inode number of the designated file. Return 0 (there is no inode number 0) in case of error. Completely off-topic, but there is a bug in this prototype: Inode numbers have type ino_t, not type unsigned long. (Ok, two bugs, if you count the fact that there can theoretically be an inode number 0. And three bugs if you count the fact that pathname should be declared as a (const char *). But the ino_t vs. unsigned long bug is really bad if you care about portability.)
- philwelch 16y ago
  You're right on all counts, and if I were writing a real filesystem rather than a class assignment I would have taken these issues more seriously. (edited to add paragraph:) Nothing I've said in this discussion should be taken as a strongly held opinion about anything--I'm rather shy about sharing these thoughts in the first place, but considered this particular example illustrative of a possibility I take seriously. You do not want to see the other horrors that particular professor of mine has perpetrated, incidentally.