Special-ness of root set: Impl. vs. data model

[Jason Kraftcheck]

On 08/31/2010 12:20 AM, Mark Miller wrote:
> 
> I can. The problem is writing code that uses the API when there are two
> kinds of sets I have to worry about; entity sets and the 'almost an
> entity set but not quite' root set. Working from a blind list of sets
> which may or may not include the special root set, I now have to test
> whether or not a given set is the root set before I pass it into
> functions accepting sets and examine the error codes they return.
>

Why would you ever want to pass the root set to some function that is going
to modify the set?

I think that the goal of an API is to provided the necessary functionality
in the most concise and easily understood way as possible.  How the root set
integrates into the data model should only be considered in so far as how it
affects the usability of the API.  I can imagine many cases where we should
probably support all set query functions on the root set.  There are many
different algorithms that one may wish to use on either a subset of the mesh
or the entire mesh.  And it is entirely harmless to do so.

However, I think that allowing modification operations on the root set is
different.  There are four basic set modification operations that could
involve the root set (other than tags, which we are allowing):
  1) destroy the root set
  2) add/remove entities from the root set
  3) add the root set to another set
  4) make the root set a parent/child of another set

I am certain that allowing the first two is a truly horrible idea.  Both
take what is a relatively benign operation for a set and define a much more
destructive behavior for the root set (delete the entire mesh or delete an
entity from the mesh.)  That is a rather cruel trap that is far more likely
to result in bugs than be of any use.  Further, allowing this would have
precisely the opposite affect of the example you are trying to avoid:

> if (set != rooSet)
>     iMesh_rmvEntFromSet()
> else
>     iMesh_deleteEnt()

In practice, if I were writing a subroutine that modified a set I would
check for the root set explicitly to avoid unintended consequences:
  if (set != rootSet)
    iMesh_rmvEntFromSet(set,ent);
  else
    return iBase_FAILURE; /* don't accidentally delete entities */

And anyone who failed to add such checks would be setting themselves up for
an unpleasant surprise.

I ordered the initial list of modification operations in order of decreasing
potential for unintended side effects.  The third item, adding the root set
to another set is guaranteed to, by definition, create a cyclical
containment graph.  While the API does not prohibit cyclical set relations,
it is typically a bug for an application to create such a containment graph.
 Further, the meaning of a set containing the root set is rather unclear.
Presumably one could just use the root set itself in place of the containing
set.  As there is no reasonable use case (that I can think of) for wanting
to do this and because there is a high probability that an attempt to do it
is a bug in the application, I don't think we'd be doing users of the API
any favors by allowing the behavior.

The final operation type (parent/child links) doesn't really have any
unintended side effects.  So I'm not strongly opposed to allowing such
functionality for the root set.  However, it still seems more likely to mask
a bug than to provide a useful functionality.  I'm not very enthusiastic
about implementing a bunch of special case code which is unlikely to ever be
(intentionally) used.

- jason