Group: Naming Inconsistencies

[kylemcdonald]

Naming inconsistencies are: poorly named methods, functions, constants, classes, and arguments.

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.

[underdoeg]

ofFbo::Settings::depthStencilAsTexture should be useDepthStencilAsTexture to be in sync with useStencil and useDepth https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/gl/ofFbo.h#L71

[ofZach]

ofGraphics::Command - non US spelling for "centre"

[underdoeg]

exitApp should be ofExitApp() https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/events/ofEvents.h#L18

[ofZach]

ofInterpolateMethod enum should start with 0 https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/graphics/ofPixels.h#L10

[ofZach]

we are using to and into to mean the same thing in ofpixels:

https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/graphics/ofPixels.h

see for example, mirrorTo and pasteInto.

[julapy]

inline ofVec2f ofVec2f::operator/( const ofVec2f& vec ) const inline ofVec2f& ofVec2f::operator/=( const ofVec2f& vec ) inline ofVec3f ofVec3f::operator/( const ofVec3f& vec ) const inline ofVec3f& ofVec3f::operator/=( const ofVec3f& vec ) inline ofVec4f ofVec4f::operator/( const ofVec4f& vec ) const inline ofVec4f& ofVec4f::operator/=( const ofVec4f& vec )

should not be using a ternary operator.

https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/math/ofVec2f.h#L337-341 https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/math/ofVec2f.h#L333-335 https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/math/ofVec3f.h#L377-379 https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/math/ofVec3f.h#L381-386 https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/math/ofVec4f.h#L308-310 https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/math/ofVec4f.h#L312-318

[justdayan]

of3dUtils.h

should ofDrawAxis be renamed to ofDrawAxes to follow ofDrawRotationAxes plural..

[NickHardeman]

QTKitMovieRenderer.h and QTKitMovieRenderer.m should be ofQTKitMovieRenderer.h and ofQTKitMovieRenderer.mm

[ofTheo]

enums should not use hex notation. and should start with = 0 and not enumerate the rest of the list: https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/utils/ofConstants.h#L371

Except for ofOrientation which needs to start at 1 for iOS compatability

[justdayan]

ofCamera.h

https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/3d/ofCamera.h#L58-61

worldToScreen screenToWorld worldToCamera cameraToWorld

should be prefixed with get

[ofTheo]

ofLoopType at top of ofConstants.h should be somewhere later and not hex notation. https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/utils/ofConstants.h#L8

[jvcleave]

ofAppBaseWindow.h

Change setupOpenGL to setup? https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/app/ofAppBaseWindow.h#L15

Remove "Window" from function names (e.g. setWindowPosition -> setPosition, this would effect every appWindow however) https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/app/ofAppBaseWindow.h#L22

enableSetupScreen/disableSetupScreen() could benefit with comment/explanation of usage https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/app/ofAppBaseWindow.h#L49

[justdayan]

ofQuickTimePlayer.h

https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/video/ofQuickTimePlayer.h#L76

allocated should be changed to b or is to mark as bool

[ofZach]

ofPolyline features hard coded default curve resolutions, ie:

int curveResolution=16

this should be a #define such as default_curve_resolution

[justdayan]

QTKitMovieRenderer.h & QTKitMovieRenderer.h

the files should be prefixed with of

[arturoc]

not sure this belongs here but why ofSetupPerspective gets an ofOrientation parameter? shouldn't we just use ofSetOrientation and ofSetupPerspective gets the orientation using ofGetOrientation? passing that parameter to setupPerspective doesn't really set the orientation and it actually has to be set to the current orientation of the screen to actually work properly

[underdoeg]

Why isBound when it is an integer?

https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/gl/ofFbo.h#L86

[ofTheo]

define name clashes

PI -> OF_PI TWO_PI -> OF_TWO_PI etc

DEG_TO_RAD - maybe we add ofDegreesToRadians(float deg) ? and slowly deprecate DEG_TO_RAD MIN -> ofMin() MAX -> ofMax()

CLAMP -> ofClamp ABS -> ofAbs

https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/utils/ofConstants.h#L311

[justdayan]

ofURLFileLoader.cpp

ofURLFileLoader::get should be should be changed to ofURLFileLoader::load and ofURLFileLoader::loadAsync

https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/utils/ofURLFileLoader.cpp#L45

[ofTheo]

What does ofFillFlag do? https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/utils/ofConstants.h#L355

[ofTheo]

What is #define OF_CLOSE (true) ??? https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/utils/ofConstants.h#L437

[kylemcdonald]

makeScaleMatrix and makeTranslationMatrix need named arguments instead of just floats.

[ofZach]

ofRenderCollection draw(ofMesh.....) would be great to have one method if at all possible (there are two now)

https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/graphics/ofRendererCollection.h#L39-45

[kylemcdonald]

ofQuaternion::length2() should be called ofQuaternion::lengthSquared() and ofQuaternion::conj() should probably be conjugate().

[ofTheo]

ofDrawBitmapMode -> ofBitmapMode https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/utils/ofConstants.h#L537

[jvcleave]

ofAppRunner.h

maybe change ofSetupOpenGL to ofSetupWindow https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/app/ofAppRunner.h#L11

OFSA could be changed to something more modern? https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/app/ofAppRunner.h#L15

[arturoc]

@justdayan ofURLFileLoader::get is called get because it uses the http GET method, at some point we would want to add POST which result would be similar to get and the load naming can be confusing, there could be a load method though that calls get and in the future if we add post it can have a method parameter but i think it's fine as it is

[ofTheo]

ofBuffer - @kylemcdonald wasn't happy with custom constructors in ofBuffer https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/utils/ofFileUtils.h#L13

[ofTheo]

why do we have ofBuffer::getText and string() operator ? https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/utils/ofFileUtils.h#L13

[arturoc]

about custom constructors: i think we should add custom constructors to every class that has a load, setup or allocate method, it's not the most usual in OF but is a pretty used pattern in c++, if you create a class inside a function only to be used in that function it's shorter and useful. it also allows for one of the most characteristic patterns in c++: http://en.wikipedia.org/wiki/Resource_Acquisition_Is_Initialization

[arturoc]

why do we have ofBuffer::getText and string() operator ?

yes we should probably deprecate that one, i added the cast operator later and didn't remove getText for backwards compatibility

[ofTheo]

ofFile::getSize returns uinit64_t - is this okay across all platforms? https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/utils/ofFileUtils.h#L142

[justdayan]

ofQTKitGrabber.mm

listVideoDevices listAudioDevices listVideoCodecs listAudioCodecs

should be:

getAudioDevices() getVideoDevices() getAudioCodecs() getVideoCodecs()

https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/video/ofQTKitGrabber.mm#L52-55

[ofZach]

ofRenderCollection:: bClearBg

https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/graphics/ofRendererCollection.h#L259

method starts with "b" should be isClearBg -- even then it's not super clear, maybe something like "isBackgroundAutoCleared()" or something is easier to understand.

[underdoeg]

ofLight::getIsEnabled -> isEnabled ofLight::getIsDirectional -> isDirectional ofLight::getIsSpotlight ->isSpotlight ofLight::getIsPointLight -> isPointLight

Also isSpotLight has upper case L but isPointlight does not.

https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/gl/ofLight.h#L47

[NickHardeman]

@arturoc "ofURLFileLoader::get is called get because it uses the http GET method, at some point we would want to add POST which result would be similar to get and the load naming can be confusing, there could be a load method though that calls get and in the future if we add post it can have a method parameter but i think it's fine as it is"

I think a load function that has a method as an argument would be ideal. Something like load(string url, string method) and loadAsync(string url, string method, string name) The reason that it is confusing is that it is not consistent with the naming of the global function, which is ofLoadURL(); https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/utils/ofURLFileLoader.h#L59

[arturoc]

all the classes that have a load*() method should have a load() instead, we can deprecate or not the old ones

[ofZach]

ofPixels::allocate param needs to be better named;

https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/graphics/ofPixels.h#L35

this should be ofPixelsFormat format, not ofPixelFormat type

[ofTheo]

https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/graphics/ofTrueTypeFont.h

charProps in ofTrueTypeFont.h -> rename?

[damian0815]

ofBaseSoundPlayer: loadSound(), unloadSound() <-> setup/close

[ofTheo]

https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/math/ofMath.h

ofRandomf() ofRandomuf()

@kylemcdonald suggests ofRandom(float max = 1);

ofRandomuf() would then become ofRandom()

Not sure about ofRandomf() though. -> ofSignedRandom() ?

Also see #224. (Taking the liberty of editing-in-line here. Christoph)

[ofTheo]

https://github.com/openframeworks/openFrameworks/blob/develop/libs/openFrameworks/math/ofMath.h

ofRadToDeg -> ofRadiansToDegrees ? ofDegToRad -> ofDegreesToRadians

or maybe its fine?

[damian0815]

ofBaseSoundPlayer <-> ofBaseVideoPlayer consistency.

setVolume, setPan should be consistent with video. setLoop should be setLoopState. getIsPlaying() should be isPlaying / isPaused after ofBaseVideoPlayer. getDuration() needs to be added. isFinished() should be added.

[damian0815]

ofBaseSoundStream <-> ofBaseVideoGrabber

listDevices should return a vector, setDeviceID should work like device selection on the ofBaseVideoGrabber.

[julapy]

ofBaseVideoGrabber::initGrabber is inconsistent with sound. https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/types/ofBaseTypes.h#L195

[julapy]

ofBaseVideoPlayer::close should be unloadSound. https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/types/ofBaseTypes.h#L227

[danomatika]

Global texture functions use "Tex" in naming while functions inside ofTexture use "Texture" ...

[arturoc]

ofBaseVideoPlayer::close should be unloadSound.

i think close it's fine, ofSoundPlayer::unloadSound should be renamed to close

[julapy]

ofRectangle::canonicalize ofRectangle::getCanonicalized ofRectangle::isCanonicalized are ambiguous.

should the process of having positive width and height be done internally/automatically?

https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/types/ofRectangle.h#L66-68

[danomatika]

There are a number of bool getters in ofTexture.h that could be renamed:

• ofGetUsingArbTex -> ofUsingArbTex
• ofGetUsingNormalizedTexCoords -> ofUsingNormalizedTexCoords
• ofGetUsingCustomTextureWrap -> ofUsingCustomTexWrap
• ofGetUsingCustomMinMagFilters -> ofUsingCustomMinMagFilters

Also missing:

• ofUsingTextureEdgeHack