Re: Special-ness of root set: Impl. vs. data model
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
participants (1)
-
Jason Kraftcheck