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

Already on GitHub? Sign in to your account

Group: Weird Globals #1549

Open
kylemcdonald opened this Issue Sep 6, 2012 · 8 comments

Comments

Projects
None yet
5 participants
Contributor

kylemcdonald commented Sep 6, 2012

Weird globals include anything that should be moved form a header to cpp file, or methods that are unnecessarily exposed, or functions that should be methods.

This is a group of issues identified during Code Review 2012. Add any relevant issues to this group, and when they are all closed we will close this issue. If an issue fits this pattern, but is more significant or not a simple fix, please add it as a separate issue.

Contributor

underdoeg commented Sep 6, 2012

Why are functions like ofSetEscapeQuitsApp or exitApp in ofEvents.h? https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/events/ofEvents.h#L16

Owner

arturoc commented Sep 6, 2012

all the events are handled in ofEvents.cpp as it contains some statics needed for both traditional callback methods and ofEvents. although the declarations could be somewhere else i think it makes sense so it's easy to find the definitions if you know where the declarations are

Contributor

NickHardeman commented Sep 6, 2012

in ofConstants.h, PI, TWO_PI, M_TWO_PI, FOUR_PI, HALF_PI, DEG_TO_RAD and RAD_TO_DEG should have OF prefix or OF namespace.
https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/utils/ofConstants.h#L311-337

Contributor

NickHardeman commented Sep 6, 2012

MIN, MAX and ABS functions should have OF prefix, ofMin, ofMax, ofAbs and shouldn't be #defines.
https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/utils/ofConstants.h#L339-345

Contributor

underdoeg commented Sep 6, 2012

If we decide to support multiple windows at some point. (And I think we will) Global functions like ofGetMouseX, ofGetMousePressed, will behave confusing or not work at all. We could define that those will always refer to the main window. But that can lead to other problems. (Does the app close when the main window is closed or not. If not, will another one take its place.. etc...) https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/events/ofEvents.h#L7

Contributor

danomatika commented Sep 6, 2012

The following global functions in ofTexture could be statics inside ofTexture since they seem to be very specific (aka GL only):

  • ofGetGlInternalFormat
  • ofGetGlFormatAndType
  • ofGetImageTypeFromGLType
  • ofGetUsingArbTex
  • ofEnableArbTex
  • ofDisableArbTex
  • ofGetUsingNormalizedTexCoords
  • ofEnableNormalizedTexCoords
  • ofDisableNormalizedTexCoords
  • ofEnableTextureEdgeHack -> could also be ofEnableTexEdgeHack
  • ofDisableTextureEdgeHack -> could also be ofDisableTexEdgeHack
Contributor

danomatika commented Sep 6, 2012

There are a number of FreeImage-based globals in ofImage:

  • ofLoadImage
  • ofSaveImage
  • ofCloseFreeImage -> should this be global? Can't it be called internally ....
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment