Note: This page is under construction and soon will move to GitHub.
Page Contributors: Michael Sherman, Ayman Habib, Jen Hicks
Header Guards are the lines like:
that surround every header file to prevent it from being included multiple times. There are two problems with OpenSim’s choice of names for the header guards:
1) They can interfere with user code, and
2) They violate the C++ standard.
1) OpenSim uses many very common class names, like “Object” and “Array” and probably “PropertyTable”. These are likely to appear in other code libraries as well, so anyone who combines OpenSim API with other libraries or their own code may have conflicts. The way you are supposed to avoid that is very simple – the header guards should be unique symbols, easily achieved by including the product name in them (that is, they should contain “OPENSIM”).
2) The C++ standard prohibits user code from using identifiers that contain double underscore (“__”) or begin with an underscore followed by a capital letter (“_P”). Those symbols are reserved for the compiler and the std:: library. OpenSim’s use of symbols like that means it is subject to conflict with the compiler, either now or in future releases or new platforms. Hopefully those conflicts would cause compiler errors, but that is not guaranteed.
One other minor issue is that preprocessor macros should typically have very ugly names with LOTS_OF_CAPS to make it obvious that they are not ordinary identifiers, and to avoid conflicts with NiceCamelHumpIdentifiers.
So OpenSim header guards should be written like this:
That matches the scheme used in Simbody and avoids all of the above problems. (Trailing underscore, but not leading, is allowed.)
Creating New OpenSim Objects
Every OpenSim object class now automatically defines a typedef “Super” that refers to the immediate parent (“superclass”) of that class. If you have to delegate something to your parent, use “Super” rather than explicitly listing the parent class, because if that changes due to future refactoring your code will still compile but be wrong! This is not a hypothetical problem -- I found bugs of that sort sprinkled around OpenSim.
Now if someone changes the class structure later so that you component’s parent changes in the header file, the code in the .cpp file (which will have been long forgotten) will automatically change its behavior.
Assignment Operators in C++
You should let the compiler automatically generate the copy constructor and copy assignment operator for your classes whenever possible. But sometimes you have to write one. Here is the basic template for copy assignment:
You run into problems if copy assignment operators are missing lines 2 and 4. Since the “copy stuff” part often begins by deleting the contents of “this”, a self assignment like a=a will fail without those lines; that is always supposed to work (and does for all the built-in and std library types). Of course no one intentionally does that kind of assignment, but they occur anyway in general code since you don’t always know where the source comes from.
If the “copy stuff” part consists only of assignments that work for self assignment, then you can get away without the test, but unless you’ve thought it through carefully you should just get in the habit of putting in the test.
Documenting your Code
If you use Visual Studio, goto Tools:Options:Text Editor:C/C++/Tabs, set tab size=indent size=4, and check the "Insert spaces" button.
Renaming Classes in the OpenSim API
Sometimes it makes sense to change the name of a class in OpenSim because the name is confusing or doesn't reflect the desired function. This seemingly innocent, and usually desirable refactoring, has some side-effects that API developers should be aware of so that changes do not break working functionality.
- Turn on JavaWrapping in CMake. You have to have Swig and Java installed.
- Build JavaWrap project to run SWIG (http://www.swig.org/, version 2.0.4)
- Run test case testContext which ends up simulating a few GUI calls.
If a class is not included in the wrapping interface file ("OpenSim/Java/swig/javaWrapOpenSim.i) then the class is likely not used by the GUI and so is safe to change, otherwise please consult with GUI developers first before renaming.
Please follow the convention that property names use “lower_case_with_underscores” as their names, while object types use “CamelCaseUpAndDownWithoutUnderscores”. That ensures no conflicts with XML tag names and makes it easy to tell a property name from an object name.
Other C++ Coding Style Suggestions
Throw and return are not functions
In C++ “throw” and “return” are not functions. It is misleading to enclose their arguments in parentheses. That is, you should write “return x;” not “return(x);”. A parenthesized expression is not treated the same as a function argument list. For example f(a,b) and return(a,b) mean different things (the former is a 2-argument function call; the latter is an invocation of the rarely-used “comma operator”).
Always use pre-increment and pre-decrement operators when you have a choice
Both pre-increment ++i and post-increment i++ are available. When you don’t look at the result, they are logically equivalent. For simple types they are physically equivalent too. But for complicated types (like iterators), the pre-increment is much cheaper computationally, because it doesn’t require separate storage for saving the previous result. Therefore you should get in the habit of using pre-increment in all your loops:
This will prevent you from using the wrong operator in the expensive cases, which are not always obvious.
Of course in cases where you actually need the pre- or post-value for something, you should use the appropriate operator.
Place “*” and “&” with the type, not the variable
References and pointers create new types. That is “T”, “T*”, and “T&” are three distinct types. You can tell because you can make typedefs like this:
Therefore you should place the “*” and “&” next to the type, not the variable, because logically they are part of the type. Unfortunately, the C language had a bug in its syntax which has been inherited by C++. A line like “char* a,b” is treated like “char* a; char b;” rather than “char* a; char* b;”, but if I write “typedef char* CharPtr;” then “CharPtr a,b” declares both to be pointers. There is no perfect solution because the language is broken. However, there is no problem in argument lists (since each variable has to have its own type). So I recommend that you simply avoid the misleading multiple-declaration form when using pointers or references. Just use separate declarations or a typedef. Then always put the “*” and “&” with the type where they belong. So argument lists should look like this:
When cleaning up classes and removing methods, if you decide to remove a method then it's necessary to remove both the prototype from the header and the implementation from the cpp file (if any). While C++ doesn't complain, leaving the prototype in the header file with no implementation anywhere causes problems for wrapping. Swig runs only on the headers and has no way of knowing if there's an implementation or not. Since the methods end up being exported, they then have to be resolved at compile time of the osimJavaJNI project.