Re: Job Interview, Did I Mess Up?
daniel_t@earthlink.net (Daniel T.) wrote (abridged):
You absolutely should treat functions that have side effects
differently than functions that don't, and you need to know which
are which.
You /can/ treat functions differently if they have no side effects, but I
don't agree you necessarily should. And it's not just about side effects.
Storing the result in a local also makes sense if the function can return
different results for the same call, or because it is slow to compute.
For example, if char happens to be a Unicode type, then tolower() would
well be quite expensive. Storing the result is the safest option which
requires least special knowledge.
I don't have strong feelings about whether or not the local should be
used. I do have fairly strong feelings about whether someone should be
criticised for using one.
I find it interesting that on the one hand, I am being lambasted
for not adding extra variables in a function even though what they
represent is already expressed, while on the other (below,) I'm
being lambasted for putting in a variable that actually
represents something that isn't expressed any other way.
The difference is that the former case isn't adding mutable state. It's
at most another name for the function result. (This would be clearer if
we added a "const", but I wouldn't bother.)
And a "result" variable is expressing what was already expressed by the
code position.
[2]
bool found = false;
while (first != last && !found)
found = (*first++ == value);
return found;
[...]
If I were required to write a function like your example (I
actually did write one recently,) my instinct would be to
write it like example 2.
I didn't expect that, because earlier you said that you wouldn't
introduce a framework of flags, and variables like "found" are what I
think Balog Pal meant by that.
[3]
while (first != last)
if (*first++ == value)
return true;
return false;
[...]
I consider example 3 fine, but with it, I have to put two
breakpoints in the function to track what it will return, and I
can't change the result during a debug run.
Although I'm happy to use debugability as an argument, it does have the
problem that different debuggers have different capabilities. I use
MSVC++. With that, having the result expressed in the code position makes
it easier to change the return value. The current code position is
identified with an icon in the left-hand column of the code, and you can
change the code position by dragging the icon. So you can drag it from
one return statement to another. It's easier to do that than to edit the
value of a variable.
This debugger also makes it easy to set breakpoints on a line, by
clicking in that column. Hence another benefit of having different return
values expressed as different lines is that it makes it easy to break
only when the result is true (or only when the result is false).
Similarly a profiler which tells you how often each line is executed,
will tell you how many true results and how many false results.
In general, I find multiple returns are easier to work with with my
tools.
However, imagine happening across a function that has 2 continues,
3 breaks (with no switches,) 4 loops, 5 returns and no documentation.
A function with 4 loops is almost certainly doing too much. Although it
sounds like a mess, adding more mutable state would not make it simpler.
Instead of having to cast a beady eye over it looking for return
statements, you have to inspect it for assignment statements instead.
Indeed, it's actually confusing to see a line like:
bool result = false;
because it is lying to the computer. The result isn't false. We don't yet
know what the result is. Similarly if we later come across:
result = true;
we still can't be sure what the result is, because it might be changed
later. We have to inspect all the code between the assignment and the end
of the function. Where-as with "return true;" we know immediately what
the result is with confidence. Once the answer is known, it's simplest to
stop.
Here you have written two functions (contains and contains_inner)
that have the exact same preconditions and postconditions.
Two functions that do exactly the same thing, with the
same algorithm, in the same program doesn't sound like a good
idea to me.
They have different purposes; they have no code in common. It's really
not that uncommon to have one layer that checks arguments and another
layer that does the work. It's a clean separation of concerns.
-- Dave Harris, Nottingham, UK.
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]