Code review comment for lp:~jhodapp/qtubuntu/qtubuntu_add-live-orientation

Hi Jim,

That is good, code is working well.

I only have a few coding style remarks, it's not written anywhere in the project but QtUbuntu respects the coding guidelines from Google (http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml):

- Not sure about the "Depends" change in debian/control, might need to be validated by ricmm.

- No need to test for orientationSensor_ validity in ~QUbuntuScreen, just delete it. "delete" takes care of NULL pointers and the sensor is stopped in its destructor.

 - Add DLOGs in OrientationReadingEvent constructor and destructor as it's done in src/modules/application/application_image.cc for instance.

 - Add DLOGs in onOrientationReadingChanged() and customEvent() at the beginning as it's done in every other methods.

 - I think using a single switch in customEvent() would make the code easier to read and maintain (also Google coding guidelines recommend to use brackets for cases, 2 spaces indentation, 4 spaces line wrap), something like that:

  switch (oReadingEvent->orientation_) {
    case QOrientationReading::LeftUp: {
      currentOrientation_ = (nativeOrientation_ == Qt::LandscapeOrientation) ?
          Qt::InvertedPortraitOrientation : Qt::LandscapeOrientation;;
      break;
    }
    case ...
  }

 - QtUbuntu uses full include paths for Qt5 classes (QtCore/QObject, QtSensors/QOrientationSensor, QtGui/QScreen, QtCore/QThread, etc). If you don't know where a class is located, "find /usr/include/qt5 -name QFoo" can help you.

 - Open curly brace should be on the same line than the last parameter of a function or the last initialization parameter of a constructor (see lines 47, 87, 98 and 143 in screen.cc). Google coding guidelines recommend Egyptian brackets (http://www.codinghorror.com/blog/2012/07/new-programming-jargon.html).

 - "Qt::ScreenOrientation QUbuntuScreen::orientation() const" should be defined in the header.

 - Bad indentation in QUbuntuScreen: QOBJECT macro and methods should be indented 2 spaces, private, public and protected keywords should be indented 1 space.

 - For pointer declarations, space is following the asterisk.

 - I'd remove the comment on line 73 in screen.cc, code is explicit enough.