Long nested namespace and source code readability & complexity

[damienmarchal]

According to the SOFA guidelines, we have the following rule:
"You must avoid the using directive in header files (.h and .inl): using namespace foo."
The reason for this rule is that doing so generates namespace pollution which increase the risk of name collision. The drawback is that the long nested namespaces prefixes are repeated all around in the .h and .inl which impact readability.

What do you think of changing the rules and allowing to use "using namespace" and 'using' in the following context:

namespace sofa
{
    namespace component
    {
         namespace engine
         {
                 namespace boxroi     // A per file namespace 
                 {
                          using sofa::defaulttype::Vec3 ;
                          using namespace sofa::whatever 
                          class BoxRoi 
                          {
                                         // Here i can use Vec3 instead of sofa::defaultype::Vec3 or defaulttype::Vec3
                          };
                 }

                using boxroi::BoxRoi ;         // declaring only the BoxRoi in the engine namespace to 
                                                           // the sofa current namespace structure.. 
         }
    }
}

With such design the names imported in the boxroi namespace by 'using' and 'using namespace' are not leacking in the engine/component/sofa namespace.

I see no side-effect of such design but maybe you do.

DM.

[matthieu-nesme]

Personally I am not disturbed by expliciting namespaces, at least the code is a bit easier to follow. 'using namespace' is somehow hiding information.

Also remember that you can add typedef inside your class.
E.g.
class BoxRoi { typedef sofa::defaulttype::Vec3 Vec3; // Here i can use Vec3 instead of sofa::defaultype::Vec3 or defaulttype::Vec3 };

Maybe is it possible to do trickier things with c++11???!

[untereiner]

Or replace typedef keyword by using keyword for the type as c++11 is allowed:
class BoxRoi { using Vec3 = sofa::defaulttype::Vec3 };

[damienmarchal]

I all,

On my side I think that the 'using' directive (the classical 'using namespace': http://en.cppreference.com/w/cpp/language/namespace) is really bad and is hiding information (It is in fact similar to the python "from xx import *"), for this reason is should be forbidden into the .h and .inl.

But it also seems to me that the situation of the 'using' declaration (http://en.cppreference.com/w/cpp/language/namespace) as well as the 'using' type alias (http://en.cppreference.com/w/cpp/language/type_alias) is really different. As they are 'explicit' they do not hide anything and we can control their visibility as python developpers are doing with the "from xx import MyVec as Vec3".

So given the fact that the latter two are:

• not hiding information
• does not leak name into namespaces
  who is to forbid/allow their use in the sofa header files and should we update the Sofa guidelines to take them into account ?

DM.

[JeremieA]

Many years ago during a technical meeting we made two decisions that were never fully implemented :

1. API classes ( in sofa::core, sofa::helper, ...) should only use a two-level hierarchy, with grouping documented as doxygen comments for example but not through deep namespaces
2. component classes (sofa::component::* ) would be moved to namespaces matching the name of the plugin in which they are, in order to match their new include path

But unfortunately it was low priority and not put into work... yet ;)

[damienmarchal]

Maybe doing

namespace sofa
{
namespace core
{
      using sofa::core::verydeeptnamespace::oldnamingscheem::AClass ; 
} 
}

Would allow to expose the old namespace prefix to the new scheme without the need to change the existing code base.

This could help during a transitional period because the existing code wouldn't need to be changed at a single time.

DM.

[JeremieA]

That's a good idea! The same could be done for the include path, temporarily keeping sofa/core/xx/yy.h which would just be a one-liner #include <sofa/core/yy.h>. Or doing the opposite, which could make merging existing codes easier (at the cost of requiring the move later, and not having the actual code in the files that are supposed to be the new correct location now).

Regarding the earlier point about using using in the class itself, I know that one previous bothersome limitation was that the syntax for the return types of methods that are not defined within the class would still require defining the complete type, leading to codes like :

• template<class T> typename T::VecCoord MyComponent<T>::mymethod(VecCoord v)
• template<class T> very::long::namespace::MyType MyComponent<T>::mymethod(MyType a)
• template<class T> typename MyComponent<T>::MyType MyComponent<T>::mymethod(MyType a)

But now with c++11, if VecCoord and MyType are typedef/using within the class, we can do :

• template<class T> auto MyComponent<T>::mymethod(VecCoord v) -> VecCoord
• template<class T> auto MyComponent<T>::mymethod(MyType a) -> MyType

A good improvement :)

[damienmarchal]

I tried the private namespace trick + 'using' to support backward compatibility in
I tried the combination of private namespace and 'using' in
https://github.com/SofaDefrost/sofa/blob/341330312c267ce5695bbd23f7ad7a8ef2b11c37/SofaKernel/modules/SofaBaseVisual/VisualModelImpl.h

Feedback welcome.

[damienmarchal]

Hi all

I would be very happy if at the STC we re-warm the decision about having a clear naming scheme.
So if some of you are interested in this topic I suggest to book for one of our cofee break next week :)

My current position on the issue is that:

• it become more and more messy as we have no 'applied' rule.

To me there is three interesting feature namespace should have each grounded by different needs:
- namespace that mimick path.
- namespace that group code logically.
- namespace that are short to write/read.

Since c++11 it is very possible to have all that with a dual namespace scheme in which one namespace match the files location. But as this namespace is in general long we have a second namespace organization that export the names in a more accessible namespace.

Ex (for a class in sofa/somehwere/again/MyClass.h):

    /// namespace (hierarchical grouping)
    namespace sofa {
           namespace somewhere {
                namespace again {
                      namespace __myclass__ {
                            /// This is a private namespace that I like a lot :) 
                            /// because we can import things we want with name leacking all around
                            /// No more typedef when they are not needed :) 
                            /// The name here are available in __myclass__ (so normally only 
                            /// in the .cpp and .h and .inl) 
                            using std::vector; 
                            using sofa::defaultype::Vec;
                            using sofa::suplercomplex::longns::OtherClass ; 

                            class MyClass : OtherClass
                            {
                                         MyCLass(Vec<3,float>&, ...) ; 
                            };
}
}
}
}

Then come the logical grouping in which the MyClass is exported into its logical group with 'using'

    /// The second namespace schema (logical grouping)
    namespace sofa {
           namespace component {
                  namespace collision {
                          using sofa::somewhere::again::__myclass__::MyClass ;
                  }
           }
    }

So generally I such approach that this increase a lot the readability of the source code and conceptually this gives a taste of how python is handling import of modules and avoid nameclash.

[hugtalbot]

@fredroy it could be nice to define the new policy in the guidelines, in this discussion and subsequently close it
are you willing to do it?

[github-actions[bot]]

⚠️ ⚠️ ⚠️
@damienmarchal
We are setting a new forum management policy: topics like this one, in which the last reply is more than 4 months old, will automatically be closed. Without further news in the coming weeks, our bot will proceed to automatic closure, thus keeping this forum clean and fresh 🌱 Thank you for your understanding

[github-actions[bot]]

⚠️ ⚠️ ⚠️
@damienmarchal
In accordance with our forum management policy, the last reply is more than 4 months old and is therefore closed. Our objective is to keep the forum up to date and offer the best support experience.

Please feel free to reopen it if the topic is still active by providing us with an update. Please feel free to open a new thread at any time - we'll be happy to help where and when we can.

[github-actions[bot]]

⚠️ ⚠️ ⚠️
@damienmarchal
In accordance with our forum management policy, the last reply is more than 4 months old and is therefore closed. Our objective is to keep the forum up to date and offer the best support experience.

Please feel free to reopen it if the topic is still active by providing us with an update. Please feel free to open a new thread at any time - we'll be happy to help where and when we can.