Git Product home page Git Product logo

Comments (4)

Jmgr avatar Jmgr commented on April 28, 2024

In my projects I always use absolute filenames when including files from external libraries, but I would find it better if the library itself would enforce this.
For me a good example of a well-designed library is Boost. Every library in Boost has its own directory (except some very old headers). I think Boost is a way better example than Qt concerning that. I think that prefixes are really obsolete and confusing in modern C++. (It also reminds me of the much feared Hungarian notation!)
As for the issue with the separation of Corrade and Magnum, I think that "using namespaces" are pure evil, so I am used to look at the documentation to know were a specific functionality resides. As for having different versions of the libraries, I suppose you could just use another prefix? I'm not sure if this is a very common use case.

from magnum.

miguelmartin75 avatar miguelmartin75 commented on April 28, 2024

I think that "using namespaces" are pure evil

@Jmgr
In my opinion, that's only the case in header files (specifically within global scope; and even in global scope in source files as there may still be name collisions).

For me a good example of a well-designed library is Boost. Every library in Boost has its own directory (except some very old headers). I think Boost is a way better example than Qt concerning that. I think that prefixes are really obsolete and confusing in modern C++. (It also reminds me of the much feared Hungarian notation!)

@Jmgr
+1 I fully agree.

Unwanted separation of Corrade (sub)library. Currently Corrade library is a first-class citizen in Magnum, i.e. the user doesn't have to think whether some class is part of Magnum or Corrade, it's enough just include it and use as if it was part of Magnum:

#include <Magnum.h>
#include <Utility/Directory.h>

using namespace Magnum;

Directory::write(...);

With this change the user would need to explicitly specify the actual project given header comes from, which is not so convenient:

#include <Magnum/Magnum.h>
#include <Corrade/Utility/Directory.h> // huh?

@mosra
It may not be "convenient" in your words, but handling directories (i.e. doing what corrade does) is not apart of the magnum library. It is apart of the Corrade library, which is required to be installed for magnum, but the user does not require to (explicitly) use Corrade in their code. As he/she may instead decide to use an alternative to Corrade, which he/she may be familiar with (such as boost). If the user wants to use Corrade, he/she should do so explicitly; the reason for being explicit is simple: it's more readable to do so, e.g. a 3rd party person observing one's code may not be sure what header file belongs to what library.

To allow multiple versions of the library be installed alongside each other, the includes would need to be installed into (e.g.) include/Magnum1/Magnum and include/Magnum2/Magnum, which is even more horrifying.

@mosra
Typically when working with SDL, it seems convention (at the moment, at least), to have the headers within two seperate directories. include/SDL (no number seems to be implicit for 1; which is convenient, since it's similar to algebra x == 1x) and include/SDL2, which seems to work quite fine. Note that to include the header files within the SDL2 library, you would write #include <SDL2/SDL.h>, which in my opinion is better than #include <SDL/SDL.h>, as it is more explicit for which SDL version you are using (which helps for looking up documentation and reading the code, as you know what version of the library you are using).

from magnum.

mosra avatar mosra commented on April 28, 2024

The using namespace is evil if it brings symbols into global namespace. In all cases I'm bringing names from one namespace to another. In Magnum.h I'm doing precisely this:

namespace Magnum {
    using namespace Corrade;
    using Corrade::Utility::Debug;
    using Corrade::Utility::Warning;
    using Corrade::Utility::Error;
}

And in my projects I'm often creating a "import header" with these contents, which is then included everywhere else, making Magnum namespace available in namespace of particular project.

namespace MyGame {
    using namespace Magnum;
}

This doesn't pollute the global namespace or cause any unsolvable naming conflicts. It is, however, entirely up to users whether they want to prefix everything with Magnum:: or not. I am strongly for using std:: for STL things, as it is short enough to not add much noise into the code, but Magnum:: is too long. There are also namespace aliases, e.g. namespace Mn = Magnum;, which might help with the noise, but on the other hand add confusion to everything.


As for the includes, my only remaining concern is that I need to move everything from src/ to src/Magnum so I can make also the inter-project includes absolute. In the distant future (when there will be some Magnum 2) I will need do the move again, from src/Magnum to src/Magnum2, even if the source changes would be more evolutional than revolutional.

This move would complicate Git history and file-specific Git logs and make src/ look like an redundant path level. Is there any way to avoid this (preprocessor options, faking include path)? There is also an option to move headers from src/ to include/Magnum and keep the sources where they are, but I would lose the source-header locality, which is very convenient when just browsing through the files.

from magnum.

mosra avatar mosra commented on April 28, 2024

I finally managed to change all the includes to absolute, the relevant commit set is from 45a10ce to 2222922 (and similarly named commits in Corrade and other projects). The changes were pretty drastic so please bear with me :-) All depending projects are also updated and the freshly uploaded documentation now reflects the "absolute state" too.

If building with MAGNUM_BUILD_DEPRECATED and CORRADE_BUILD_DEPRECATED it is still possible to use non-absolute includes (so some backwards compatibility is preserved), with non-deprecated build only absolute includes are allowed. To avoid issues it may be needed to recreate the build dir and also update FindCorrade.cmake and FindMagnum.cmake.

from magnum.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.