search

libglui / glui

GLUI is a GLUT-based C++ user interface library which provides controls such as buttons, checkboxes, radio buttons, and spinners to OpenGL applications. It is window-system independent, using GLUT or FreeGLUT.
Other
194 stars 82 forks source link

Code Tidy-up: Use private static methods for primary GLUT callbacks #68

Closed nigels-com closed 7 years ago

nigels-com commented 7 years ago

Improved object-orientation, hiding internals.

nigels-com commented 7 years ago

Done.

m-7761 commented 7 years ago

Since you directed me here, I don't understand why these are under GLUI_Main. If they are static why not move them into glui.cpp in the global namespace?

The convention I use that I believe works best is to whenever possible put procedures in the CPP file they are needed in, making them "static" by default (avoid the "anonymous namespace" because it's too attention grabbing) and name them with the name of the CPP file with the extension removed and an underscore where the . would be. If I want to work in a namespace I use the name of the file with the _cpp as the name of its namespace. (So things done in its namespace can not use the prefix.)

This way if a state needs to be 'extern' the "static" can just be replaced with "extern" and that marks it as shared with another CPP file somewhere, and there will be a matching extern in those files where it's shared. And the "extern" state will be named after the file it originates.

In this case, these callbacks would still be called glui_x because they'd live in the "glui.cpp" file. It's a good system. Think of the TUs as big master objects. In any case, parking these under GLUI_Main just clutters its namespace, so clients are presented with a menu of red herrings when they use auto-complete facilities.

nigels-com commented 7 years ago

Ah, the auto-complete can't distiguish between private and public?

m-7761 commented 7 years ago

I set mine to not distinguish. IDEs can't even change formatting preferences on a per project basis. How do they know if data is interesting to the programmer or not? Private members are interesting to the developers working with (typing) them.

So no. And generally they are interesting too. Clutter should be minimized. If something is only used in one file and is static then it has no earthly business existing outside of that file. The linker has to consider everything that's not static to the translation-unit. You think oh it doesn't matter, but a million things that don't matter funnily in concert do matter, so it's a matter of developing good habits.

nigels-com commented 7 years ago

It seems to me a normal sort of thing to have static private functions for callback purposes, assuming that they're truely private. The alternative is polluting the global namespace, which you seem to be proposing for the sake of code completion convenience. I really don't mind much either way, but it doesn't seem much to do with good habits.

m-7761 commented 7 years ago

The alternative is polluting the global namespace

You misunderstand. The glui.cpp file isn't the global namespace. Where its code begins is the end of the global namespace as far as pollution is concerned. This is code 101. The whole idea of pollution is to keep things out of the CPP (translation-unit) that it doesn't need or want. Well, glui.cpp is the CPP. And everything in it is completely outside the scope of all other translation-units by definition.

Think about it.

nigels-com commented 7 years ago

Sure, it's out of the header. But it's still the global namespace from the perspective of the .cpp

nigels-com commented 7 years ago

So speaking of namespaces, next on my list was to move GLUI to a namespace and remove the GLUI_ prefix from all the class names. In that arrangement I suppose "polluting" the GLUI namespace is less of a concern. But I think I still prefer the to be private to the relevant class.

m-7761 commented 7 years ago

So speaking of namespaces, next on my list was to move GLUI to a namespace and remove the GLUI_ prefix from all the class names. In that arrangement I suppose "polluting" the GLUI namespace is less of a concern. But I think I still prefer the to be private to the relevant class.

I think that's good and will make it easier to reintegrate what work I do. But the GLUI::Main namespace is a different namespace from the GLUI namespace. Adding members that could be static/internal to a TU increases the linker's workload and is just untidy. (Something's scope should be as narrow as its use. Period.)

If these private members used an underscore prefix (which I know is controversial but still beats the hell out of any alternative) it would be less of a nuisance. But it would still be pointless and misleading to file them as static private members. Something programmers (people) seem unable to appreciate at large is that the absence of something is every bit as meaningful as the presence of something. (That is to say for example if you put parentheses around every term in every expression then you are reducing the meaning of parentheses, like the little boy who cries wolf.)

nigels-com commented 7 years ago

Can we compromise and change them to private with _ prefix?

m-7761 commented 7 years ago

Can we compromise and change them to private with _ prefix?

Sure. If it feels better :) I don't really see a value in exposing it. I guess in a debugger a user can see what functions GLUI has assigned to them (these are just GLUI's own fixed callbacks right?) and maybe jump to them through the memory inspector.

Off-topic: Today I saw some GitHub action: https://github.com/nanoant/CMakePCHCompiler/pull/11

That reminds me, I downloaded the GitHub Desktop app, but neglected to install it. I found the edit function that lets you do edits/commits in the browser. That's pretty nifty for small changes I'll admit. The pull-request workflow seems more logical there, and I got the impression that forking is the normal thing if you don't have write-access. It defaults to doing a pull-request via a branch; but I think it would make more sense to default via a fork (or to use common sense and see if the logged in user can even edit the branches.)

(I had to get precompiled headers working with COLLADA-DOM because its generated headers are pretty insane; especially since I switched them over to using templates to do crazy things (every value has a pointer-to-member to itself and so can statically talk to its container.) The first PCH (GCH) was 1.8GB and it is only the header description of parts of two schemas. Granted I regret installing 64-bit Cygwin and I just removed the debug information, so next rebuild it should get smaller.)

ctype.h goes crazy with macros. It uses U,L,N,S,P,C,X,B to define numbers. When you start using prefixes you're bound to run into trouble eventually, but the GCC system headers really take the cake. Still I really like using _ and __. Nothing else works so well. It's stupid that C reserves those for the system. Especially since the end of the "Hungarian" debate. (God help us all.)