Re: Review: Add config.sanityCheck


Malcolm Humphreys <malcolmh...@...>
 

On 21 Dec, 2010,at 02:23 PM, Jeremy Selan <jere...@...> wrote:

Your idea would also work. Not sure if either is better.

sanityCheck() was inspired by OpenEXR, which has a function of the
same name, and performs a similar action. Guess I just had that model
in mind when writing this.

Are you against exceptions?
 
Not really.

Would you ever expect to call OCIO::GetCurrentConfig() and get back a pointer to an invalid profile? Throwing an exception from inside GetCurrentConfig() feels sensible, while throwing in sanityCheck() just feels a bit strange ie.
...
OCIO::ConstConfigRcPtr config = OCIO::GetCurrentConfig();
try {
    config->sanityCheck();
} catch(...) {
...

vs

OCIO::ConstConfigRcPtr config = OCIO::GetCurrentConfig();
if(config->sanityCheck(msg)) {
...

vs

try {
    OCIO::ConstConfigRcPtr config = OCIO::GetCurrentConfig();
} catch(...) {
...
if(config->sanityCheck(msg)) {
...

But I do like the idea of a sanityCheck type of function for when we are building a profile in code and need to check it is vaild.

If we went with your idea, why an enum instead of a bool?
"bool isValid()"

I would expect users to only want to know if there is an error or not.
Under what circumstances would a user be returned the "UNKNOWN" value?
The reason I have 3 states internally is to know if i've computed the
value, and if so, if it succeeded or not. The internal 'unknown'
state is just an implementation detail, the user is not aware of it.
 
a bool is even better.

.malcolm

Join {ocio-dev@lists.aswf.io to automatically receive all group messages.