Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

namespaces and relative header paths. #8

Open
memo opened this issue May 24, 2013 · 11 comments
Open

namespaces and relative header paths. #8

memo opened this issue May 24, 2013 · 11 comments

Comments

@memo
Copy link
Member

memo commented May 24, 2013

I'm raising two issues here, both quite distinct but related.

  1. I know namespaces are a hot topic. Here's a problem I just ran into, try using ofxLeapMotion and ofxMultitouchPad in the same project. They both have a class called 'Finger' so won't compile. Calling the classes ofxLeapMotionFinger and ofxMultitouchPadFinger are really ugly solutions. This is what namespaces were invented for so I suggest we encourage addon developers to use namespaces. Luckily, Leapmotion has its own namespace, however there is a 'using namespace Leap' at the top of ofxLeapmotion header file which creates ambiguity. It is an easy hack fix, (remove the 'using', and explicitly state Leap::Finger instead of assuming it via the using). So I suggest:

    • addon developers always use namespaces if their class names are likely to be common names
    • addon developers never have 'using namespace ....' in their headers, but always explicitly state the classes (e.g. Leap::Finger)
  2. There is another common and similar problem. Having header files with the same name. E.g. Finger.h, Control.h etc. When doing #include "Finger.h" you can run into ambiguity problems if different addons have a Finger.h. Using ofxLeapMotionFinger.h or ofxLeapMotionControl.h is again an ugly solution. I've started using relative paths to the addons folder, makes readability better I think. e.g.

    include "ofxMyAddon/src/Finger.h"

    For this to work '../../../addons' needs to be added to the header search path. If add-on writers have guidelines to follow this convention then adding '../../../addons' to the project once will ensure ALL addons will always work, and we won't need to add different header search paths for each addon (unless the addon is using a 3rd party lib, e.g. oscpack, which has it's own requirements). In fact perhaps this could become part of the example projects and template / PG?

@bilderbuchi
Copy link
Member

pinging @elliotwoods as he brought this up previously

@elliotwoods
Copy link

great post!

I agree with 1
but i disagree with 2

my method is to lay the files out like:
addons/ofxGraycode/src/ofxGraycode.h
addons/ofxGraycode/src/ofxGraycode/other files

the user then adds the ../../../addons/ofxGraycode/src to their include path
ofxGraycode.h then includes files using #include "ofxGraycode/OtherFiles";

@bilderbuchi
Copy link
Member

what about .h files within addons/ofxGraycode/src/ofxGraycode/, though? e.g. in Payload.h, you just #include "DataSet.h", so we're back to square one...

also, I have to admit I don't like such a pretty convoluted file layout, it feels kinda forced. I like memo's solution #2 better. One question though would be, if we add ../../../addons to the header search path, don't we then pull all the available addons into the search path, and not only those we actually use in the projects? don't know how the different IDEs behave here.

@elliotwoods
Copy link

from Payload.h i can just do #include "DataSet.h" because the 2 files are in the same folder (the src/ofxGraycode/ folder is not in the header search path, only src/ is)

when i looked into this last year for my addons ofxGraycode/src/ofxGraycode/* felt cleaner than:

  1. ofxGraycode/libs/ofxGraycode/include etc
  2. having to do #include "ofxGraycode/src/ofxGraycode.h" from user code

it just makes simple sense, you've got 1 file ofxGraycode.h which the user includes (as per normal), and everything else is ofxGraycode/something.h.

(to note : ofxGraycode was the first addon where i started adopting this pattern. ofxCvGui2 / ofxMachineVision demonstrate the pattern a little better)

@bilderbuchi
Copy link
Member

from Payload.h i can just do #include "DataSet.h" because the 2 files are in the same folder (the src/ofxGraycode/ folder is not in the header search path, only src/ is)

but if some other addon (or OF proper) has a DataSet.h in the header search path, we would get an ambiguity/collision, right?

@memo
Copy link
Member Author

memo commented May 25, 2013

@bilderbuchi
yes exactly, this is one of the problems that I'm trying to address.

Problem 1:
If there are two headers which have the same name (e.g. Camera.h, Control.h, DataSet.h) that belong to two different addons, and somewhere in one cpp, one of these files are included with no path, i.e. #include "Camera.h", then there could be an ambiguity, even if they are in the same folder. It's not the user which should be using full paths for the headers, chances are they will only include the main addon header file (E.g. #include "ofxGraycode.h") which is almost guaranteed to be unique. It's the files inside the addon which are more likely to cause collisions, (e.g. Camera.h, Control.h, DataSet.h etc). So the addon source files need to use relative paths I think.

Problem 2:
Wouldn't it be nice if no matter what IDE you're using (xcode, cb, vs etc), you could just add the files of an addon to your project, and not have to worry about adding anything to the header search paths! (assuming you don't have 3rd party libs and dependencies).

4 different people recommended 5 different methods! Here's my summary:

@kylemcdonald (addresses problem 1 I think, not problem 2)

  • main header goes in addons/ofxMyAddon/src/ (as it currently does)
  • *.cpp go in addons/ofxMyAddon/libs/ofxMyAddon/src/ and *.h in addons/ofxMyAddon/libs/ofxMyAddon/include/
  • add '../../../addons/ofxMyAddon/libs/' to project header search paths (for every single addon)
  • add '../../../addons/ofxMyAddon/src/' to project header search paths (for every single addon)
  • addons include their own headers by doing: #include "ofxMyAddon/include/aHeader.h"
  • users include addon main header as normal: #include "ofxMyAddon.h"

@elliotwoods (doesn't address either problem, but could potentially address problem 1. definitely not problem 2).

  • main header goes in addons/ofxMyAddon/src/ (as it currently does)
  • all other *.cpp and *.h go in addons/ofxMyAddon/src/ofxMyAddon/
  • add '../../../addons/ofxMyAddon/src/' to project header search paths (for every single addon)
  • addons include their own headers by doing: #include "aHeader.h" ? (this doesn't solve the ambiguity issue)
  • users include addon as normal: #include "ofxMyAddon.h"

@arturoc option1 (addresses problem 1, not problem 2)

  • main header goes in addons/ofxMyAddon/src (as it currently does)
  • rest go in addons/ofxMyAddon/libs/ofxMyAddon/include/ofxMyAddon/aHeader.h
  • add '../../../addons/ofxMyAddon/libs/' to project header search paths (for every single addon)
  • add '../../../addons/ofxMyAddon/src/' to project header search paths (for every single addon)
  • addons include their own headers by doing: #include "ofxMyAddon/aHeader.h"
  • users include addon as normal: #include "ofxMyAddon.h"

@arturoc option2 (addresses problem 1, not problem 2, introduces new problem of mega long filenames!)

  • main header goes in addons/ofxMyAddon/src (as it currently does)
  • all other *.cpp and *.h go in same folder
  • add '../../../addons/ofxMyAddon/src/' to project header search paths (for every single addon)
  • addons include their own headers by doing: #include "aHeader.h"
  • users include addon as normal: #include "ofxMyAddon.h"
  • headers are called ofxMyAddonAHeader.h to avoid ambiguity (!)

@memo (addresses both problems)

  • main header goes in addons/ofxMyAddon/ (this could be just a dummy header which only includes the relevant headers with relative paths).
  • all other *.cpp and *.h go in addons/ofxMyAddon/src/ (as they currently do, or organize into subfolderes)
  • add '../../../addons' to project header search paths (once for all addons)
  • addons include their own headers by doing: #include "ofxMyAddon/src/aHeader.h"
  • users include addon main header with relative path: #include "ofxMyAddon/ofxMyAddon.h"

@elliotwoods's solution seems nicest to me (apart from mine :) most functional for the least radical change. However, addons including their own headers without a path could cause ambiguity. So even if they are in the same folder I think they should still include their own headers using " ofxMyAddon/aHeader.h". This minor mod makes it address problem 1.

However, with all of these solutions, you still need to add each addon to the header search paths. This is the step I was trying to avoid. Seeing as we know that all addons go in the addons folder, I was trying to find a solution which involved 'just working' out of the box. Doesn't matter what IDE you use, you add the files of an addon, and hey presto it works! And the few simple steps I summarised above does this :)

@elliotwoods
Copy link

@bilderbuchi - false
the files of the other addon wouldn't see DataSet.h because DataSet.h is not in the header search path (for either addon, so both addons wouldn't see the file of the other addon).
Payload.h can see DataSet.h because it is in the same folder, but i agree with you and memo that a more consistent approach would be to internally address includes using paths relative to the search path (i.e. ofxGraycode/DataSet.h) , but in both cases the compiler's header search path stays the same, and conflicts between addons are not an issue (the compiler looks local to the file before checking header search path, and addon B's files are invisible to addon A and vice versa).

@elliotwoods
Copy link

one extra thing to note for newcomers here (which doesn't affect @memo's summary above and i think you 2 are both aware of) is that in visual studio every header include must be relative to the header search path (i.e. adding the h file to your project does not automatically make it available).

i think in many cases that ../../../addons is already in the project search path (i've seen that in some project settings for a while now by default).

i'm +1 for an addon's main include to mostly be a gateway include to all the rest of the files, e.g. https://github.com/elliotwoods/ofxCvGui2 (this addon takes namespaces a little to the extreme, with multiple nested namespaces).

Can I introduce a couple more problems:
3. Linking necessary external libs (generally fine on xcode with drag/drop method, but not other platforms)
4. Including library headers
5. Changing the file structure of an addon breaks old projects which use that addon
6. Addon must be recompiled for each project which use it (not much of a problem, but kind of unnecessary)

For me, issues 3-5 kind of put a downer on the #include "ofxAddon/ofxAddon.h approach, and 6 is important too for big addons.

My solution to 3,4,5, 6 in Visual Studio is to have:

  1. A project for the addon which includes all source files and links libs, and builds a static lib included in app project (the user adds this to their solution, and links the project from their app project)
  2. A .props file with all necessary project settings to use the addon (the user just adds this to their project).

This requires a couple more setup steps but saves a lot of time in the long run (especially if you're developing an addon across multiple apps), but also removes all the existing steps of using an addon (adding files to project, manually editing project properties). I'm not sure how well this approach can be rolled out to other IDE's.

Great conversation here @bilderbuchi + @memo !
I'm glad to see people finally talking about this properly.
I'm all for working towards a better standard for this and willing to adopt a new pattern if it

@memo
Copy link
Member Author

memo commented May 26, 2013

@elliotwoods
"The files of the other addon wouldn't see DataSet.h because DataSet.h is not in the header search path (for either addon, so both addons wouldn't see the file of the other addon)."

This isn't the case for XCode. As soon as a header is added to the XCode project, any file can include it. If you add two headers with the same name to different folders, a source file in another folder can include the header file (i haven't yet understood the logic as to which one is actually included, but one of them definitely is).

So perhaps always using paths is safer.

I think yours is definitely the nicest of the solutions which address problem #1. However it doesn't address problem #2.

I have no idea how to address the new problems you mention. The two options you mention are the only things I can think of too, however to make this cross IDE compatible could be a ball ache.

@bilderbuchi
Copy link
Member

Seeing as we know that all addons go in the addons folder,

this is not necessarily true anymore, at least the new makefile system can already include addons from whatever location. Granted this is a non-default and probably not often invoked behaviour.

What we also have to think about is that any solution is ideally robust against addons not conforming rigidly to the style. It's unfortunately not very realistic to get all addon devs to modify their addon's structure according to this template, as the different addon structure approaches presented here already show.

@arturoc
Copy link
Member

arturoc commented May 26, 2013

The solution i like most is Elliot's. For problem number 2 i think we shouldn't solve this at the file system level but through project files. For 008 we are going to include this new file for addons (optionally) https://github.com/openframeworks/openFrameworks/blob/develop/addons/ofxOpenCv/addon_config.mk

That solves the special search paths for makefiles in linux, android and osx and for every platform when using the PG. What we could have is some kind of application or even web page that generates a project file per addon so to include an addon you somehow include that file which already has the correct search paths, external libraries... I guess in xcode this would be an .xcconfig file so when you drag and drop the addon that file gets included to and does the necesary configuration. For visual studio if suppose it should be a project file, it could even compile a library per addon if necesary. For windows codeblocks we could probably move to makefiles the same way it works on linux already

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants