Coding Conventions

[teamikl]

This is suggestion for refactoring: Coding Conventions

I pick up the check list of summary. read the original document for long details.

---

C++ Feature

• [x] Don't use exceptions
• [ ] Don't use rtti (dynamic_cast)
• [x] Every QObject subclass must have a Q_OBJECT macro

  Casting

• [x] Don't use dynamic_cast, use qobject_cast for QObjects or refactor your design
  • NOTE: in general C++ programming, down cast is a time to consider re design class to use virtual method.

    Compiler/Platform specific issues

• [ ] Be extremely careful when using the questionmark operator.

  Aesthetics

• [ ] Prefer enums to define constants over static const int or defines.
• [ ] When reimplementing a virtual method, do not put the virtual keyword in the header file.
  • On Qt5, annotate them with the Q_DECL_OVERRIDE macro

---

other points I founds:

• memory management
  • [ ] stack or heap (new/delete)
  • [ ] QScopedPointer ... consider they really have to be located in heap? or can be on stack
  • [ ] QObject heap tree ... some of object missing parent? leak and destructor may not be called
• [x] avoid unnecessary copy of data -- use reference instead
  • iteration variable can be const reference if read only iteration
• [ ] the difference of "C++11's for" and "Qt's foreach"
• [x] avoid returning static variable -- it's same as global variable

[teamikl]

@elieserdejesus Can you add a something tag to these programming topics? Users does not have interest internals.they may want filtering.

[teamikl]

dynamic_cast

// e.g.
dinamic_cast<Type>(obj)->method();

• dynamic_cast may raise std::bad_cast (for reference type)
  • un-handled exception may break qt's main event loop.
  • no dynamic_cast for reference type used in JamTaba?
  • NOTE: i grep dynamic_cast, but if there was traditional C-style cast as downcast then I don't know. it's not search-able.
• dynamic_cast may return NULL (for pointer type)
  • then "method" to null pointer.

replace all dynamic_cast to qobject_cast ?

No. it's case by case.

I am not sure what situations you needed the dynamic_cast. Most case qobject_cast can be just replace, several cases can avoid cast.

• when need down cast then consider virtual method
• about the layout() cast, get the layout object directly instead of widget's layout

[elieserdejesus]

I will discuss some questions:

stack or heap (new/delete) Always prefer stack (pointerless programming) to avoid pointer problems. If a pointer can be avoided let's avoid it. If not let's try encage the beast in some smart pointer to avoid manual "delete" code. I'm using QScopePointer in some places. I decide to QScopedPointer to avoid mix Qt and STL. We are using Qt eveytime, why not use Qt smart pointer too? STL performance is better? Show me the numbers... he he. What your code profiler is saying? :)

QScopedPointer ... consider they really have to be located in heap? or can be on stack I always consider the stack option first. But using stack we loose the polimorphism, so in many places we need a base class pointer storing a derived class instance. So the rule is: prefer the stack, if not possible (polimorphism is the only case in my mind at moment) use QScopedPointer to the base class.

QObject heap tree ... some of object missing parent? leak and destructor may not be called I'm assuming when a widget is added in a layout it is "parentized" automatically. I'm wrong? If I'm not wrong the cases of "non parent" widgets are solved when these widgets are added to some layout.

avoid unnecessary copy of data -- use reference instead We need discuss what "unecessary copy" means. Copy one byte is a unnecessary copy? And 2 bytes? And 1024? My opinion: micro optimizations are wast of time in a application like Jamtaba. We have just 2 points to worry about performance: audio processing and widgets drawing. All other micro optimizations are waste of time. Compilers do a good job managing return by value or parameter by value. My propose is: Use const reference in parameters if the function is called frequently. When I say "frequently" I'm saing "many times per second". In all other cases (99%) avoid the verbose syntax of const references and use simple and fast enough parameter by value. The decision to return by value or not is more complex because is context dependent. The returned instance life cycle can be analized in each situation. But again, return by value (pointerless) is always my first option.

iteration variable can be const reference if read only iteration This is good, is better ensure the "read only" behavior. But I have to admit I'm lazzy for check this detail. Please correct me in code revisions.

the difference of "C++11's for" and "Qt's foreach" I don't know the differences, and I suppose is minimal. I use some c++ for loops in the Jamtaba beginning, but I swith to Qt foreachs avoiding mix loop syntax styles.

avoid returning static variable -- it's same as global variable I wrote some bad code for ninjam protocol (and for other things too :)) when starting Jamtaba. In that moment I was not used to C++, so I wrote some very dirty things. For example, I'm returning a static istance for each ninjam message, if I remember correctly. I do this ridiculous thing (now I know how ridicuolous it is :)) to avoid create and return a new instance for every message. In that moment I'm worried with performance problems in create instances. Totally bullshit! This is not a performance botleneck. The solution is simple return by value. Return a pointer is a bad thing in this case because this will force the callers to destroy the new instance, a bad thing. Returning smart pointers is too much, very verbose and not worth. Return by value solve the problem, is fast enough (we are not receiving 500 ninjam messages per second), avoid pointer problems and eliminate the static shit. Another point of static usage in the code are the singletons. I know, I know, a big shit! :) But if I remember correctly we have just 2 or 3 singletons, is not difficult refactoring to use better design.