search

sandflow / regxmllib

Convert MXF to XML: RegXML (SMPTE ST 2001-1) tools and libraries
BSD 2-Clause "Simplified" License
35 stars 14 forks source link

Remove `using namespace` directives from header files #124

Closed CausticYarn closed 6 years ago

CausticYarn commented 6 years ago

Several header files declare XERCES_CPP_NAMESPACE_USE, which is equivalent to using namespace xercesc. This is considered bad practice because it forces the consumer of the library to pollute the global namespace (e.g. https://stackoverflow.com/questions/4872373/why-is-including-using-namespace-into-a-header-file-a-bad-idea-in-c) which can easily cause name clashes in a medium or large scale project. Using declarations in .cpp files is not an issue (as long as they are after any includes).

Attached is a patch which uses fully qualified names for the header files and moves the using directive to the .cpp files.

xerces_namespace.patch.txt

palemieux commented 6 years ago

Sounds good. Can you create a pull request?

palemieux commented 6 years ago

@CausticYarn I created PR #127 based on the patch above. I am thinking it is unusual to use fully-qualified names in .h files but not in .cpp files. What about using fully-qualified names everywhere? Any reason not to?

CausticYarn commented 6 years ago

For .cpp files, I believe it's just for style and readability whether or not to use fully qualified namespaces, since it doesn't affect code that uses the file (unlike those declarations in the header).

I personally find it helpful to use fully qualified names wherever possible, as it really helps someone unfamiliar with the code to understand which tokens are defined by the library itself and which are part of third party includes.

palemieux commented 6 years ago

@CausticYarn Can you review PR #127 when you have a chance?