[Kyle's list] 8 - Source code formatting

theo, could you add your changes to the github wiki? https://github.com/openframeworks/openFrameworks/wiki/oF-code-style we’re trying to collect all of the specifics into one place for reference.

Okay just updated it. I noted the changes to the Qt style in the section and changed the examples above to match.

Some of the stuff at the end that relates to git / pull requests could be pulled into a separate page at some point.

https://github.com/openframeworks/openFrameworks/wiki/oF-code-style

Thanks Theo.
I have some questions/corrections:
Never use a single space after a keyword and before a curly brace.
That should be reworded - sometimes there has to be a space after a keyword I think, virtualvoidfunc() is not valid syntax.

No Initialisation lists: I’m interested in the reason behind this. What’s the advantage that outweighs the performance hit of an additional constructor call per member variable? Clarity? Or are there other drawbacks to init lists?


Regarding the virtual keyword riddle from some posts back, I confirmed my suspicion: virtual keywords cascade down a hierarchy automatically, but for clarity it’s recommended to keep them anyways. I say we drop that part of the Qt style.
http://stackoverflow.com/questions/6397356/what-does-virtual-keyword-when-overriding-method/6397364#6397364
http://stackoverflow.com/questions/2963965/why-is-virtual-optional-for-overridden-methods-in-derived-classes

Sorry I meant that sentence as in (after a keyword && before a curly brace) :slight_smile: so: virtual void func(){ instead of virtual void func() {

Mainly clarity and precedence.
I understand that its a little more efficient in principle. Though anytime someone says A is more efficient than B I ask them to show it in practice. Its amazing what compilers can do and often I’ve found the compiler can optimize away the differences. If someone can make an example app that demonstrates a significant difference then maybe it is something we should look at for types which are used a lot within a project ( ofColor, ofPoint etc ).

In general we prefer code that is easy to read unless there is a significant benefit to the contrary. Another important point is that we want the differences between OF code and Processing code to be pretty minimal, so we will always be hesitant of things that result in a big syntax difference when there isn’t significant benefits to performance/stability/legibility.

I’ll defer to you on that one :slight_smile:

@Whitespace: I cleared that up, is this what you meant: https://github.com/openframeworks/openFrameworks/wiki/oF-code-style/-compare/e49a2a775cf0018606d61ab944a85336cc915528…f088b686afffedb4700cd7e304ab43a7743a4477

@Init lists: Thanks, sounds reasonable.

@virtual and inheritance: I took it out.

Sorry I meant that sentence as in (after a keyword && before a curly brace) :slight_smile: so: virtual void func(){ instead of virtual void func() {

Could you explain why? (For me the latter is clearer)

Whenever one assigns variables inside the constructors bodies ( personName = name; ) this means that those variables were initialized using their default constructor before, so that time was wasted as we don’t need their default value. By using initialization lists, ( Smth::Smth(string name) : personName(name) ) we are initializing and readily assigning the variable with our desired value.
Also, variables that are const or references must use this way.

We are providing a framework, a middleware that other people will build their programs on top of. This means that although the speed doesn’t have to meet NASA’s needs, it should still be fast. Also, maybe the difference in the times when using initialization lists or body assignments is minimal in an example program but if one uses OF to generate a 10^X particle system, it might cripple performance and let’s not forget the mobile devices which are usually slower.

In general we prefer code that is easy to read unless there is a significant benefit to the contrary. Another important point is that we want the differences between OF code and Processing code to be pretty minimal, so we will always be hesitant of things that result in a big syntax difference when there isn’t significant benefits to performance/stability/legibility.

I understand the desire of OF being easy to use and hence making it more like Java. But it is not Java and again, we’re talking about a framework, the code people do on top of this is another thing and everyone is free to use C++ in the fashion they feel more comfortable with.

Cheers,

If you want to put together some test cases that solidly illustrate the differences it would be very helpful to see :slight_smile:
If there is a noticeable difference even at the 100K level then the initialization lists could be something we switch to. I just don’t trust when people saying things are faster without testing. ‘Optimizing code’ without proof doesn’t help anyone and just opens up the possibility of more bugs and decreased legibility.

I understand the need for const and references too so as noted in the wiki currently it can be used in those (rare) cases.

In terms of other syntax stuff we will always favor what the current de facto practice is unless there is a very strong reason otherwise. 90% of the code in the core API is currently the way described in the wiki. So we’re not going to start small preferences like void func() { or class capitalization without there being a strong reason for it.

I see your point but I wasn’t talking about weird optimization stuff.
I think initialization lists are pretty common and easy to understand. As for it being fast for real, well, part of my work in MeeGoTouch involved taking care of performance and I can tell you that the smallest thing can cripple performance for some use-case.
In the case of initialization lists, you can easily see that if you avoid constructing an object with a value that will have no usage at all will waste time to almost the double.

Anyway, even though I’d like to see things done this way, in this thread I was caring more about the source code format than the performance (though I’m surprised to see this lack of care for performance in a framework of this kind).

To say there is a lack of caring about performance is quite an overstatement!
We care deeply about performance.
I think maybe our relative backgrounds might explain the differences?

OF’s background is as a creative tool more designed for interactive installations and generative software ( typically run on Desktop / Laptop systems ). So when we think of performance we are thinking more about optimizations in regards to drawing, opencv type operations, video performance, pixel access etc. We spend a lot of time to try and make these things as fast as possible and its a big priority for us as a whole.

Its only recently that we have released OF for iPhone and Android. In that respect we don’t have as much experience designing frameworks for mobile usage. Our perspective is still quite desktop-centric in some regards. So please forgive us if we are a little skeptical as we might not always be thinking about things in the same context. Optimization for mobile systems is going to be a bigger part of OF going forward and there is a lot to be done with how things currently are in that regard.

If it seems to be a big issue for mobile performance then I don’t think anyone would object to moving towards initialization lists going forward.

I understand that Theo and I didn’t mean to offend you or anything. It’s just that the non-usage of initialization lists for the sake of code clarity impressed me.

Hey, no problem :slight_smile:
I think its good for people to challenge things like this ( it wasn’t a long time ago OF was using char * and c arrays instead of strings and vectors ).

We also definitely need more perspective coming from the device / embedded computing realm, so its really good to have this sort of input!

with regard to this question by theo in the wiki:

Currently OF is heavily biased towards tabs, is there something we could run as part of git commit that could convert tabs to spaces? Otherwise we stick with tabs. People contributing to OF should set their tab and indent width to four spaces.

Several options:

Based on the built-in K&R, I made a code style for Eclipse using the current state of the Wiki document. I left unspecified options as-is. Enjoy.

openFrameworks-eclipse-codestyle.xml.zip

For the record:

someType.h:

  
#pragma once  
  
class someType {  
public:  
	  
	//someType(){};    // default constructor with no implementation  
	/*someType(){      //default constructor with implementation  
		x = 0.0;  
		y = 0.0;  
		z = 0.0;  
		a = 0.0;  
	};*/  
	~someType(){};  
	someType(float _x, float _y, float _z, float _a) : x(_x), y(_y), z(_z), a(_a) {};  
	  
	void setX(float _x) {x = _x;};  
	void setY(float _y) {y = _y;};  
	void setZ(float _z) {z = _z;};  
	void setA(float _a) {a = _a;};  
	  
	void setup(float _x, float _y, float _z, float _a) {x = _x; y = _y; z = _z; a = _a;};  
	  
	float x;  
	float y;  
	float z;  
	float a;  
  
};  
  

testApp.h:

  
#include "testApp.h"  
#define ITERATIONS 100000  
static someType * sArr[ITERATIONS];  
//--------------------------------------------------------------  
void testApp::setup(){  
	  
	int start, attempts, totalIL, totalSU, totalMS, avgIL, avgSU, avgMS;  
	start = totalIL = totalSU = totalMS = avgIL = avgSU = avgMS = 0;  
	  
	attempts = 10000;  
	  
	for (int tries = 0; tries < attempts; tries++) {  
				  
		start = ofGetElapsedTimeMicros();  
		  
		for (int i = 0; i < ITERATIONS; i++) {  
			sArr[i] = new someType();  
			sArr[i]->setup(1.0, 1.0, 1.0, 1.0);  
		}  
		avgSU += totalSU = ofGetElapsedTimeMicros() - start;  
		//printf("Total time for %i iterations = %i microsecs\n", ITERATIONS, totalSU);  
		for (int i = 0; i < ITERATIONS; i++) delete sArr[i];  
		  
		start = ofGetElapsedTimeMicros();  
		for (int i = 0; i < ITERATIONS; i++) {  
			sArr[i] = new someType();  
			sArr[i]->setX(1.0);  
			sArr[i]->setY(1.0);  
			sArr[i]->setZ(1.0);  
			sArr[i]->setA(1.0);  
		}  
		avgMS += totalMS = ofGetElapsedTimeMicros() - start;  
		//printf("Total time for %i iterations = %i microsecs\n", ITERATIONS, totalMS);  
		for (int i = 0; i < ITERATIONS; i++) delete sArr[i];  
		  
		start = ofGetElapsedTimeMicros();  
		  
		for (int i = 0; i < ITERATIONS; i++) {  
			sArr[i] = new someType(1.0, 1.0, 1.0, 1.0);  
		}  
		avgIL += totalIL = ofGetElapsedTimeMicros() - start;  
		//printf("Total time for %i iterations = %i microsecs\n", ITERATIONS, totalIL);  
		for (int i = 0; i < ITERATIONS; i++) delete sArr[i];  
		  
	}  
	  
	printf("Average time for %i iterations:\ninit lists %f microsecs\nsingle setup %f microsecs\nmulti set %f microsecs\n", ITERATIONS, (float)avgIL/attempts, (float)avgSU/attempts, (float)avgMS/attempts);  
	  
}  

Results:

Default constructor with no implementation

Average time for 100000 iterations and 10000 attempts:
init lists 5662.999023 microsecs
single setup 5678.848145 microsecs
multi set 5673.958984 microsecs

Default constructor with implementation

Average time for 100000 iterations and 10000 attempts:
init lists 5638.051758 microsecs
single setup 5651.916504 microsecs
multi set 5647.956055 microsecs

Test machine: late 2010 MacBook Pro 2.8Ghz i7 8Gb Ram
There is no (relevant) efficiency gain if the init’d types are integral (see http://www.parashift.com/c+±faq-lite/ctors.html#faq-10.6) mileage will vary…

…I wrote a much longer rant about how dismayed I was by the way this got discussed – with an amusing anecdote about the time i tried to google '() : ’ the first time I encountered an initialization list (both an argument for and against it’s inclusion in the core perhaps?) - but I’ll spare you, and myself…

I guess init lists kind of pushed this over into discussing issues beyond the scope of the thread (“Source code formatting”) – grammar meets functionality.

It would be amazing (and I am totally sincere here) if we could spend as much time as has just been spent on tabs, spaces and braces to discuss each and every one of: const, namespace, vector/array, try/catch/for, exception vs bool, encapsulation, polymorphism (instance vs define), data types (strict const for use in STL; struct vs class), function delegation and dispatch, why it’s a testApp, POCO vs Boost vs specialised libs for events, networking, file functions and all the other oF whispers.

In the meantime I’d love to hear more about:
* What is meant by CLARITY? (for beginners/users and/or developers/advanced)
* What is meant by PRECEDENT? (“the way it’s been done for ages” vs “best-practice”)
* What is meant by EFFICIENT? (at write/read/learn/teach-time, compile-time, implementation-time, run-time, art-time or bug-time)

And I’d love to hear how these overlap/contradict in relation to the two (or more?) aspects of the core:
* The external API (ie., things that are going to be used a lot in “testApp”)
* The internal PARADIGM (the general approach to making the things that are used a lot by the things that are used a lot in the “testApp” :slight_smile:

And lastly, did we decide: single or double indent in headers?

Much respect (and not a little trepidation),
M

Hi gameover,

What are you trying to say with your previous comment and test? I don’t see the relation and I don’t see the usefulness of your test.
Anyway, I just took a quick look, maybe I’m missing something.

I also don’t see the point of googling for “() :”. Take a look into Scott Meyers’s “Effective C++” and check what he says about init lists. Also, look at big C++ libs like Qt and check its usage.

Anyway, again, I think this is not directly related to source code formatting and I’d like us not to deviate for the thread’s subject here.

Cheers,

Maybe things relating to c++ code style in the core of OF could be discussed in a seperate thread?
Its a hot topic and one that comes up often especially from the more hardcore C++ heads in the community.

const correctness, initialization lists, templates, namespaces etc.

I started a new thread to discuss these things here, with a little background from why things are the way they are ( think puberty ):
http://forum.openframeworks.cc/t/of-coding-practices–prev-c+±in-the-core…-/7537/1

good idea, theo, it’s better to keep that out of this thread.
In the meantime, how do we go forward from here regarding the source code format?

@gameover thanks for posting that - I always love that sort of stuff :slight_smile:
In regards to your questions I would love if you could kick things off here: http://forum.openframeworks.cc/t/of-coding-practices–prev-c+±in-the-core…-/7537/1

the eclipse plugin sounds really useful - I would like to get arturo’s opinion on the best way to proceed.
ie if this should stay as just something which is a guide that people should follow or if it is implemented in a more aggressive way via a commit hook script / cron job.

I think maybe breaking out the wiki could be useful

Sections on:

code formatting
coding practices
naming conventions
github structure and pull request practices
git tips

would be useful to populate.

We could also put these as links in a single page called “Contributing to OF”.

That way if anyone has any questions we can just point them to that page.

in any case, the coding style has to be finalized, and the last remaining items in “proposed” be merged in or discarded.
the wiki structure/content you outline is already mostly available in the two github wiki pages here https://github.com/openframeworks/openFrameworks/wiki, so that shouldn’t take too long to restructure/flesh out.

I’ve just added the suggestion of using “git add -p” to the coding style:

https://github.com/openframeworks/openFrameworks/wiki/oF-code-style
(at the bottom)