When to check for validity of parameters, and when not to

[TommyKaneko]

Recently, I have been getting an error while using this library entirely due to my fault. I have been getting BAD_ACCESS errors, on the point when Model is destroyed and released. It's a pretty bad error, as it's difficult to find out where the issue arose.

It turns out that I was directly adding a Material object from one model directly to another. So the model was releasing another model's material. Sketchup C API doesn't have a problem with doing this through SUModelAddMaterials. There are also other ways to get the same error by adding layers from another model.

@thomthom - do you have a view on whether the C Wrapper should be checking such errors made by the user? If we do check, it seems quite a performance hungry task. If we don't, these kinds of errors will be very difficult to diagnose for the user.

[thomthom]

Maybe it's possible to check if a material already have a parent? And such a check could be done in an assert - which will only be active in Debug builds, but stripped out in Release.

[TommyKaneko]

Yes, I think assert may be the way to do it. So far, I have been following the philosophy of "use assert in your own code, use exceptions for libraries". Perhaps this isn't the correct approach after all.

[thomthom]

The C++ STL have a whole bunch of extra checks in debug builds. And the compiler also do extra work for memory allocations etc.

If desired one could wrap the assert in another macro that checks a flag - allowing the user to disable it. But I think this might be worth to have on by default. You're not the first one trying to add entities across model boundaries.

[TommyKaneko]

Cool, I think I will go with the assert.

I'm not so confident with macros, so I will avoid that for now. Instead, I can think of useful standard methods for making these kinds of checks (something like Model::material_exists() ), that can be wrapped in an assert()