MADEAPPS / newton-dynamics

Newton Dynamics is an integrated solution for real time simulation of physics environments.
http://www.newtondynamics.com
Other
936 stars 182 forks source link

Use C++ namespaces instead of "nd" prefix #229

Open jonesmz opened 3 years ago

jonesmz commented 3 years ago

Why are functions and classes, such as https://github.com/MADEAPPS/newton-dynamics/blob/master/newton-4.00/sdk/dNewton/ndBodyParticleSet.h being prefixed with "nd". That's what namespaces are for?

JulioJerez commented 3 years ago

yes, but name spaces are a suggestive thing. some people like them and soem other people prefer the prefix, I am in teh later group.

jonesmz commented 3 years ago

You are polluting the global namespace with your symbols. This is strongly undesired by a lot of projects that might want to use Newton.

If you put everything into a namespace, any project that wants to use your code can pull your symbols into their preferred namespace, or the global namespace, with "using namespace newton".

But projects that don't want prefixes like "nd" cant do anything about it. They are forced to use your prefix.

If you want to have "nd" in front of everything for your code, consider making a macro. E.g. #define NEWTON_PREFIX(symbol) nd##symbol so that consumers of your code can specify their own prefix if needed.

iSLC commented 3 years ago

A lot of other projects do this. Bullet does it with bt, Box2d does it with b2 and so on. At this point I'd rather say that its because it allows a somewhat C-like interface when built as a dynamic library/shared object. Symbol names are cleaner when exported. A namespace may introduce name mangling which may differ from compiler to compiler. If he wants to do it this way, then let it be. It has purpose even if we don't see it beyond our current needs.

I suppose one could argue that a namespace may be added as a conditional macro. But is fine as it is so far.

jonesmz commented 3 years ago

Name mangling happens with C++ code, regardless of the use of namespaces.

If you want a C interface, make an actual C interface.

Hedede commented 2 years ago

I haven't actually seen a physics engine that doesn't do this. Havok uses the hk prefix, Bullet uses bt, PhysX uses Np etc

jonesmz commented 2 years ago

And everyone else jumping off of a bridge means we should as well?

It's trivial to macro-ize this to allow for the consumer of the library to use a prefix or a namespace, at their choice.

#ifdef USE_CPP_NAMESPACE
#define NEWTON_NAMESPACE(symbol) nd::#symbol
#else
#define NEWTON_NAMESPACE(symbol) nd#symbol
#endif

But inflecting global symbols on the consumer of the library is not polite.

Hedede commented 2 years ago

I don't see it as a big deal if everything is correctly prefixed. It doesn't really "pollute" anything. Name conflicts are unlikely, and it won't show up in the code completion unless you type the prefix.

I think it is better to keep prefixes than to "macro-ize" everything. Namespaces are of course better IMO, but it is better to spend the effort elsewhere.

JulioJerez commented 2 years ago

Yeah, of all of the reasons to reject a library, the name prefix is the least important that I can think up. What people has to realized is that these projects ha been going for almost 20 years. At the time not all platforms suppted cpp, an nane spaces was a new thing to cpp back them. Trying to retroactively apply this kind of changes is a real big job that does not justify the effort. Maybe for newton 5 if we ever made one.

On Tue, Mar 29, 2022, 4:46 PM Hudd @.***> wrote:

I don't see it as a big deal if everything is correctly prefixed. It doesn't really "pollute" anything. Name conflicts are unlikely, and it won't show up in the code completion unless you type the prefix.

I think it is better to keep prefixes than to "macro-ize" everything.

— Reply to this email directly, view it on GitHub https://github.com/MADEAPPS/newton-dynamics/issues/229#issuecomment-1082473941, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB6EPJBZU4JLV4LNXXUFHTLVCOI6DANCNFSM4YKNQLDA . You are receiving this because you commented.Message ID: @.***>