Re: Software landmines (loops)

[jtc@dimensional.com (Jim Cochrane)]

In article <Ez0D7y.DCG@yc.estec.esa.nl>,  <duncan@yc.estec.esa.nlx> wrote:
>In article <6t4dmi$rhp@flatland.dimensional.com>,
>Jim Cochrane <jtc@dimensional.com> wrote:
>>Of course, if the conditions being checked in the code below are really
>>pre-conditions - that is, it can be considered a coding error that the
>>function is called with one of these conditions true, then it would be
>>better coded as:
>>
>>void Worker::do_something (Tree *top)
>>{
>>    assert (top != NULL && top->child != NULL && ...);
>>
>>    _error_code = really_do_something(top->child...->child);
>>    if (_error_code != 0) // or whatever value means non-error
>>    {
>>	_error_occurred = true;
>>    }
>>    else
>>    {
>>	_error_occurred = false;
>>    }
>>}
>>
>>Where the assertion would be documented as a pre-condition of the function
>>specification.
>>
>>Then a client would do:
>>
>>    // ensure the pre-condition for root
>>    worker.do_something (root);
>>    if (worker.error_occurred())
>>    {
>>	//report/handle the error with worker.error_code() ...
>>    }
>>
>>[Changed to an OO style, since the discussion is occurring on OO
>>newsgroups.]
>>
>>The difference, of course, is that the checking for null pointers becomes
>>part of the function specification rather than being explicitely coded (the
>>assertion will probably be turned of in the production release).  This is
>>the difference between design by contract and defensive coding.
>
>
>The first 'Commandments' for coding were:
>1. Thou shalt use single entry/single exit routines.
>2. Thou shalt guarantee all pointers before dereferencing.
>
>As mentioned before, the system was made up of 7 sub-systems,
>all running as separate processes, and communicating via
>shared memory. Hence there was a lot of 'redundant' checking.

I understand that things are not black and white in the "real world" and
that compromises sometimes need to be made.  (For one thing, the fact that
C was used here, which has no exception handling mechanism, is a factor in
what path to take.)  However, I think if I was in this situation, I would
ask a few questions, such as:

What are the error handling requirements in the case where a pointer is null?
If one of these errors does occur, is this a coding error, a bug?
If so, where would the source of this bug likely be?
If one of these errors occurred, will the program still be able to run
correctly?  In other words, can the program recover from the error, or
should it report the error and terminate in order to not cause a problem,
such as corrupted data?
Who (what module(s) or routine(s)) is responsible for building the tree so that
the required nodes were not null?  If the answer is that no-one was
responsible, can the design be changed so that this responsibility can be
assigned to a particular module or modules?
Who (what module or routine) is responsible for setting things right (if
that is possible) if an error does occur?
Can the fact that a certain depth of the tree is required be considered a
contract that must be established by some (direct or indirect) client
routine, even if it is in another process?
Can the design be changed so that, even if assertions cannot reasonably be
used, the structure is less awkward?  (At the very least, it seems, the
original "do_something" function could be structured to check for the error
and report it first, rather than nesting the check of each node, as the
original did:

    if (null_descendant (root, required_depth, &null_depth))
    {
	//report that error occurred at depth null_depth and deal with it
    }
    else
    {
        really_do_something(top->child...->child);
	...
    })

I suppose my main point is that rather than simply following an edict, it
is important to ask questions like these to find out if there might be a
better way of doing things.  If this was done, and the decision to proceed
as you described was done for a good reason, then that is basically all you
can ask.

>
>Yes, you could use 'assert' but seeing as the error checking
>had to go into the production code anyway, there was little point.
>
>Yes, the pre-conditions could have been checked before the
>routine was actually called, but now you have moved knowledge
>of where in the tree data structure the relevant information
>lives into the caller. When you consider that some of the paths
>through the tree involved following 6 or more pointers, that's
>a lot of encapsulation and data hiding that has been lost.
>All of the code for determining which error has occurred has
>also been moved from the routine to the caller...
>
>These are all tradeoffs that need to be made. I believe that
>there are various 'good practices' which should be included
>in an ideal world, but until I find employment in the ideal
>world, it's going to be a case of making these tradeoffs, and
>some of the 'good practices' may not be applied in all cases.
>
>Of course it helps if you are aware of the 'good practices'
>in the first place so that you know what you are missing.

Definitely - both because you will likely be able to apply these practices
in the future, and, even in the case where such a compromise is made, this
knowledge will still allow you to do a better job (write more correct code,
etc.) within the context of the existing environment.

>
>Cheers
>Duncan
>
>This is my article, not my employer's, with my opinions and my disclaimer!
>--
>Duncan Gibson, ESTEC/YCV, Postbus 299, 2200AG Noordwijk, The Netherlands
>Tel: +31 71 5654013   Fax: +31 71 5656142  Email: duncan@yc.estec.esa.nlx
>To avoid junk email my quoted address is incorrect. Use nl instead of nlx.


-- 
Jim Cochrane
jtc@dimensional.com