Re: Review: Add config.sanityCheck


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

On 23/12/2010, at 5:38 AM, Jeremy Selan wrote:
Either of our approaches will work, of course, but I'm still inclined
to go with the exception route.

A bunch of calls in OCIO can throw exceptions (the common one being
config->getProcessor(...) ), so most of the time you'll already have
try / catch blocks. You can usually thus add a "sanityCheck" call in
a single extra line.

If we instead added: bool isValid(const char * msg)
this requires a separate code block to explicitly check it, probably 3
or 4 lines of code. Which isn't a problem, just an observation.

I'm also not the biggest fan of functions with two return arguments.
Consider the python API... If we go the exception route, it ports
naturally. If we use an isValid call, would it return the tuple
"(bool, str)"? People would probably expect it to just return a bool.
yeah I guess you would either need to do that or have some config.getErrorMsg() function.

Perhaps you object to the name 'sanityCheck'? (Which I admit is a bit
weird). How about 'validate', instead?
No I think it's ok esp if people are used to the ilm code.

The ilm code does call header.sanityCheck when using InputFile (), OutputFile(), TiledInputFile(), TiledOutputFile().

OCIO::GetCurrentConfig() will always return a valid profile object.
It currently will throw an exception only iff $OCIO is defined, but
points to a profile that has parse problems.

I'd prefer to not enforce 'auto validation' after loading at the library level.
I would say throwing inside sanityCheck is fine then.

If you think having a pointer to a profile which has problems is needed for something else then not throwing from in GetCurrentConfig() is also ok.

.malcolm

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