roboticslab-uc3m / questions-and-answers

A place for general debate and question&answer
https://robots.uc3m.es/developer-manual/appendix/repository-index.html
2 stars 0 forks source link

C++ namespace convention #15

Closed PeterBowman closed 7 years ago

PeterBowman commented 7 years ago

As work on #2 goes on, we'll find out that former TEO-specific code is intented to target a wider range of robot platforms, thus rendering the omnipresent teo:: namespace quite inappropriate for common libraries. Proposed solutions:

PeterBowman commented 7 years ago

A new, catch-all namespace was proposed by @jgvictores: roboticslab::. It might seem to be a bit lengthy; however, per Google's C++ Style Guide - General Naming Rules:

Give as descriptive a name as possible, within reason. Do not worry about saving horizontal space as it is far more important to make your code immediately understandable by a new reader. Do not use abbreviations that are ambiguous or unfamiliar to readers outside your project, and do not abbreviate by deleting letters within a word.

Although it doesn't concern namespaces explicitly (Function names, variable names, and filenames [...]), it might give a valuable pointer on how to approach this question. Remember that long namespaces can be shortened with the help of C++ namespace aliases:

namespace rl = roboticslab;
namespace teo = roboticslab::teo;

More examples and some background: http://foonathan.net/blog/2016/01/26/namespace-alias.html.

jgvictores commented 7 years ago

Added checklist above to keep track of repos affected/done.

rsantos88 commented 7 years ago

Done it! You can find the changes in develop branchs:

PeterBowman commented 7 years ago

Compilation was broken at kinematics-dynamics due to the gait repository not being prepared for the namespace switch. This was fixed at roboticslab-uc3m/gait@4fc2c91 and roboticslab-uc3m/kinematics-dynamics@bda0441. @munozyanez please keep in mind that any repository which uses gait's public interface (e.g. gaitcontrol?) must be updated accordingly, or just keep pointing at the current local gait HEAD (avoid git pull && git submodule update) until that happens.

munozyanez commented 7 years ago

Ok, thanks a lot for the work and the warning, @PeterBowman , i will keep in mind and fix the codes.

rsantos88 commented 7 years ago

thanks @PeterBowman

jgvictores commented 7 years ago

Thanks @rsantos88 and @PeterBowman for this!

@rsantos88 I'm seeing we'll have some trouble merging your fork of https://github.com/roboticslab-uc3m/openrave-yarp-plugins. We'll work on it together. :smile:

jgvictores commented 7 years ago

Added Document at https://github.com/roboticslab-uc3m/best-practices task above, to do before closing issue.

jgvictores commented 7 years ago

Done at https://github.com/roboticslab-uc3m/best-practices/commit/72fa3adf9147d64aa5dfd1780ae13b52cb3820a6.

PeterBowman commented 7 years ago

yarp-devices is still using the old namespace, but I guess we'll catch up later due to ongoing work (too many branches to synchronize everything at once).

rsantos88 commented 6 years ago

done it here!

jgvictores commented 6 years ago

Did on yarp-devices here [thanks @rsantos88 !!]