On 10/21/2010 11:45 AM, Jason Kraftcheck wrote:
On 10/21/2010 01:30 PM, Carl Ollivier-Gooch wrote:
The -real- issue is that there are internal checks that should be done to help API users with debugging. This is why we return errors. Yes, this is something the compliance tests check for, but that's not why they're there.
We return errors when things fail. That is an obviously well accepted programming practice. Failure to return an error flag of some kind when an operation fails is a bug (regardless of whether or not one is in a "release" mode.) That is distinct from helping users with debugging, which should be an implementation issue.
But those error returns are precisely the debugging aid to which I refer. If the iMesh spec says that an error code should be returned when someone tries to iterate over all faces of topology prism, then users should get that error code when they make that silly mistake. Same goes for other behavior the spec says is erroneous. This is different than returning failures for internal errors, which, as you say, an implementation should also do.
And yes, the implementation that's used is practice should be the one that's tested. What I'm saying is -specifically- that the -test- can be disabled in production mode. By this time, presumably the API user has code that is functioning properly and all those tests will pass anyway, so they're a waste of time. The only exception, I suppose, would be differences in behavior for different input, but those strike me as likely to be rare.
If the implementation is unusable in "testing" mode because it is doing some O(n^2) checking operation then have we really helped the developer?
No. But I don't know that mandate any error checking that's that expensive, at least with good implementation.
I don't know about you, but when I use #ifndef NDEBUG, I -only- put things in those blocks that are pure tests with no side effects. The code that produces useful results (i.e., the production code) goes outside these.
Is there something you're trying to imply? :)
Not at all, but if you think the shoe fits... :-)
The debug-mode code does a lot of work on the side without affecting any output. That's the programming style I think makes sense here, too: write production code and decorate it with the necessary checks, some or all of which will be disabled in production code, without affecting what the production code does.
That's a great coding practice. However, I a) don't think it is appropriate for this particular case and b) don't see why you consider it part of the iMesh API rather than an implementation choice that we made.
Checks for conditions that the iMesh spec says should return an error don't fall into the category of implementation choices, IMO. You seem to be saying the same thing when you talk about appropriateness for this case. Recognizing that some checks may be expensive (hopefully not n^2!), we've currently given implementations the option of not checking everything in production code. This may be something we need to consider, or at least make a statement like "all tests that are constant time must always be done". This is probably most of them, actually...
So no, I don't think this is the beginning of a slippery slope. And I certainly disagree that there's anything involved that's focused specifically at compliance tests.
So if users never enable our "strict conformance" mode and we don't otherwise test for this does it really matter that the compliance test checked this and that our implementation passed?
I suppose not in a practical sense. But in this case, users who are developing against your implementation may be doing things that aren't supported by other implementations. This is something we should avoid wherever we can. Coming back to the present case, we seem to have two choices: 1. Keep the present semantics and (to keep interoperability) mandate the check. 2. Change the current semantics, rendering the check irrelevant. If we're going to do this, we face the issue of supporting both directed and undirected graphs, presumably. Here the interoperability problem would be limited to a transition period when we have some implementations that don't fully support undirected and/or cyclic graphs. I think I could support cyclic directed graphs by removing a check. Undirected graphs could be really easy or could require significant re-write; I'm not sure without dissecting the code. Carl -- ------------------------------------------------------------------------ Dr. Carl Ollivier-Gooch, P.Eng. Voice: +1-604-822-1854 Professor Fax: +1-604-822-2403 Department of Mechanical Engineering email: [email protected] University of British Columbia http://www.mech.ubc.ca/~cfog Vancouver, BC V6T 1Z4 http://tetra.mech.ubc.ca/ANSLab/ ------------------------------------------------------------------------