3D Modeling

[Johan]

Using const in all the right places is called "const correctness", widely accepted to increase maintainability and readability. In my opinion, a "must-have" in industrial strength programming.
Spatial Corp doesn't seem to have this policy - why? (does ACIS depend on legacy libraries?).

For example, in api_get_faces the examined entity is read-only according to the documentation. Well, if the entity pointer was declared const, the code would speak for itself. There would be no need to check the documentation - it would be a compile-time guarantee that the entity is not modified, i.e. no unwanted side effects.

The api, as it is now, prevents me from being const correct myself - when having a const entity that I need to keep unchanged in a code section I cannot call this method (at least not without ugly cast hack). I find this sad. Any plans on fixing this in a future version?

Best wishes

Johan

[asdf]

Dear Johan

I work here at Spatial.  I agree const correctness is very important and find myself grumbling about various areas in ACIS where the code is not const correct.  I cannot speak to whether we will schedule work to fix this.

By way of explanation/apology: ACIS has been developed over a period of 20 years.  During that time, the C++ language itself was in rapid flux.  Moreover, programming conventions were not widely agreed upon.  As a result, we were inconsistent internally with API signatures.  At this point, C++ has settled down quite a bit, the standard training for C++ is a lot more consistent, and it is obvious that being const correct is important.  I am pretty sure (well hopeful anyway) that all new APIs will be const correct.  The question here regarding fixing it is a cost benefit issue.  I don't really know how long it would take for people to fix all of the instances where we violate const correctness.  There are hundreds (thousands?) of APIs.  Each time we fix an API signiture it could ripple downwards to the internals.

It seems like there is a clear benefit for ease of use though. We'll have to see what happens.

Eric

[Johan]

Hello Eric,

Thanks for the explanation, I can understand that programming conventions have changed a lot during 20 years.

I think then it may be best to start fixing the internals and go "upwards". The good thing is that adding const will not break any code calling it (only downwards like you say), so the risk will be minimal. Thus, adding const to the lowest level will only make the compiler complain in cases with "logical" constness (reference counting, caching, logging, etc that needs to change to mutable). 

Adding const might be faster than one might think, and the quality of your code will improve too (sometimes const also allows the compiler to make optimizations for better performance). I strongly recommend it if you can afford it.

I wish good luck, and hope for the best.

Johan

[Entity]

Hi Johan,

A further comment - despite the documentation labelling api_get_faces as Read Only, there are actually cases in which it makes changes to a model.  We do lazy instantation of patterns in ACIS, and querying all faces on a body can trigger this instantiation.  This can be turned off by setting the include_pat argument to PAT_NO_CREATE or PATH_IGNORE (as you perhaps already knew).

Regards,

John

[Johan]

Hello John,

Thanks for your reply.

Yes, but laziness is manageable too. For example, check out Ken Bloom's suspension class at http://stackoverflow.com/q/3374743 for a general solution to lazily-initialized members regarding this matter.

Eric said that all new APIs should be const correct, but I don't see how that will happen as long as your internals does not support const. In my opinion it's better to fix sooner than later. My wish that you're happy to consider such a project.

Johan

[Johan]

Just a quick follow up...

I can see that the effect has been corrected in the documentation of R23, which is a good thing (although it's a bit vague and should read "is guaranteed to create" instead of "can create" - at least I believe this is the case).

However, the const correctness violation still remains!

For example, you can just add an overloaded function with "const ENTITY* ent" for the read-only variants (PAT_NO_CREATE and PAT_IGNORE) with the array of const pointers to entities as output parameter. This addition won't break any existing code using api_get_faces and would be an improvement!

Furthermore, it's also recommended to do one thing at one level of abstraction, avoiding side effects when possible. In your case it would be possible to have a separate function for expanding patterns that can be called before api_get_edges when desired. In my experience, lazy instantation should be used for internal objects when it doesn't cause any public visible changes to a class (logical constness with mutable members), so in your case it's a bit confusing in my opinion.

Best wishes

Johan