Gianni said:
There was no decl in the post. Does it return a char * or a std::string?
If it returns a char * does the return value need to be freed ? If so
that would be bug number 5.
in my opinion you should simply assume that the code that is not given
is correct. Otherwise, i could say that in the lookupName there could be
also the error 6, 7, 8, and maybe 9. It doesn't make sense....
That's the one I was referring to.
Please answer below the text you're quoting, otherwise you can generate
misinterpretations.
int main(int argc, char *argv[]) {
const char *name = NULL, *c = NULL;
if (argc >= 2)
c = argv[1];
It does not parse the arguments correctly. Should you return when
argc <= 1 spouting an error message ? What should happen if you have
more messages ?
You knows nothing about the semantic of the program. In these
situations, just assume that everything you don't know actually make
sense. The argument parsing in this program is not broken.
It's broken.
More than one argument and those arguments are ignored. That's a bug in
my rulebook and if you were to ever work in my shop it would soon become
a bug in your rule book as well.
What's wrong with ignoring arguments? For me, a bug is something that:
- makes the program crash
- doesn't make the program compile (but I would call it error as Ian
pointed out)
- makes the program act differently from what I want. That is, if
nobody tells me that the program shouldn't ignore input arguments, I
assume that this is legal. It may seem a rather rigid reasoning, but
this is an interview question, it's not a real program, in which case I
would ask what is the program for, before making assumptions.
Correct parsing should be:
if (argc < 2 )
{
std::cerr<<"Need ???NAME??? parameter to command "<<argv[0]<< "\n";
return 1;
}
you decided that there should be at least a parameter? and based on
what? You know nothing about this program, and for me it seems like it
produces a result even without parameters. And, as far as we both know,
it may be the correct (expected) result.
if (argc > 2) {
std::cerr<<"too many parameters passed to command "<<argv[0]<<"\n";
return 1;
}
again. You are imposing restrictions on your own account.
...
as it stands, if there are no parameters to the command, "getName" is
passed a NULL - probably not a good idea.
or probably a precondition on lookUpName, I would say...
It depends on whose rule book you pick.
You made good comments about pieces of code that are questionable, and
that's good. The only thing is that you weren't required to do so, but
just to find the errors.
That's IMHO, of course...
probably, if the interview context would have been appropriate, after
having pointer out the two error I'd have spent some words on the parts
of code that are "weak" in terms of behaviour, that are more or less the
ones that you described (but there would be a lot, for example that it's
so bad mixing char* and std::string, and that there is no reason not to
use only std::string, make the interfaces uniform, etc.).
Regards,
Zeppe
