Update Coding convention regarding conversion operators and use of auto keyword

author Kimmo Hoikka <kimmo.hoikka@samsung.com>

Fri, 14 Sep 2018 16:55:29 +0000 (17:55 +0100)

committer Kimmo Hoikka <kimmo.hoikka@samsung.com>

Fri, 14 Sep 2018 17:08:51 +0000 (18:08 +0100)
author Kimmo Hoikka <kimmo.hoikka@samsung.com>
Fri, 14 Sep 2018 16:55:29 +0000 (17:55 +0100)
committer Kimmo Hoikka <kimmo.hoikka@samsung.com>
Fri, 14 Sep 2018 17:08:51 +0000 (18:08 +0100)
diff --git a/docs/coding-convention.html b/docs/coding-convention.html

index 79bbf03..1acc2f1 100644 (file)
--- a/docs/coding-convention.html
+++ b/docs/coding-convention.html
@@ -41,97 +41,7 @@ function toggleVisibility( button, obj )
  </HEAD>
  <BODY>
  
  </HEAD>
  <BODY>
  
-<H1>General</H1>
-  <P>
-    Here's a few pragmatic programmer guidelines to follow (<A HREF="http://www.codinghorror.com/blog/files/Pragmatic%20Quick%20Reference.htm">Web version</A>)
-    <H3>Details <input type="button" value="Hide" onclick="toggleVisibility( this, 'general_details' );"/></H3>
-    <ARTICLE class="detail" id="general_details">
-      <UL>
-      <LI><B>Care About the Software, Care about your API users and end users</B><BR>
-            Why spend your life developing software unless you care about doing it well?
-            Turn off the autopilot and take control. Constantly critique and appraise your work.
-      <LI><B>Don't Live with Broken Windows</B><BR>
-            Fix bad designs, wrong decisions, and poor code when you see them.
-            You can't force change on people. Instead, show them how the future might be and help them participate in creating it.
-      <LI><B>Remember the Big Picture</B><BR>
-            Don't get so engrossed in the details that you forget to check what's happening around you.
-      <LI><B>DRY - Don't Repeat Yourself</B><BR>
-            Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.
-      <LI><B>Eliminate Effects Between Unrelated Things</B><BR>
-            Design components that are self-contained. independent, and have a single, well-defined purpose.
-      <LI><B>There Are No Final Decisions</B><BR>
-            No decision is cast in stone. Instead, consider each as being written in the sand at the beach, and plan for change.
-      <LI><B>Fix the Problem, Not the Blame</B><BR>
-            It doesn't really matter whether the bug is your fault or someone else's—it is still your problem, and it still needs to be fixed.
-      <LI><B>You Can't Write Perfect Software</B><BR>
-            Software can't be perfect. Protect your code and users from the inevitable errors.
-      <LI><B>Design with Contracts</B><BR>
-            Use contracts to document and verify that code does no more and no less than it claims to do.
-      <LI><B>Crash Early</B><BR>
-            A dead program normally does a lot less damage than a crippled one.
-      <LI><B>Use Assertions to Prevent the Impossible</B><BR>
-            Assertions validate your assumptions. Use them to protect your code from an uncertain world.
-      <LI><B>Use Exceptions for Exceptional Problems</B><BR>
-            Exceptions can suffer from all the readability and maintainability problems of classic spaghetti code. Reserve exceptions for exceptional things.
-      <LI><B>Minimize Coupling Between Modules</B><BR>
-            Avoid coupling by writing "shy" code and applying the Law of Demeter.
-      <LI><B>Put Abstractions in Code, Details in Metadata</B><BR>
-            Program for the general case, and put the specifics outside the compiled code base.
-      <LI><B>Always Design for Concurrency</B><BR>
-            Allow for concurrency, and you'll design cleaner interfaces with fewer assumptions.
-      <LI><B>Don't Program by Coincidence</B><BR>
-            Rely only on reliable things. Beware of accidental complexity, and don't confuse a happy coincidence with a purposeful plan.
-      <LI><B>Test Your Estimates</B><BR>
-            Mathematical analysis of algorithms doesn't tell you everything. Try timing your code in its target environment.
-      <LI><B>Refactor Early, Refactor Often</B><BR>
-            Just as you might weed and rearrange a garden, rewrite, rework, and re-architect code when it needs it. Fix the root of the problem.
-      <LI><B>Design to Test</B><BR>
-            Start thinking about testing before you write a line of code.
-      <LI><B>Abstractions Live Longer than Details</B><BR>
-            Invest in the abstraction, not the implementation. Abstractions can survive the barrage of changes from different implementations and new technologies.
-      <LI><B>Coding Ain't Done 'Til All the Tests Run</B><BR>
-            'Nuff said.
-      <LI><B>Use Saboteurs to Test Your Testing</B><BR>
-            Introduce bugs on purpose in a separate copy of the source to verify that testing will catch them.
-      <LI><B>Find Bugs Once</B><BR>
-            Once a human tester finds a bug, it should be the last time a human tester finds that bug. Automatic tests should check for it from then on.
-      <LI><B>Sign Your Work</B><BR>
-            Craftsmen of an earlier age were proud to sign their work. You should be, too.
-      </UL>
-    </ARTICLE>
-  </P>
-
-  <H2>Avoid Tight Coupling</H2>
-    <P>
-      Always choose the loosest possible coupling between entities. In C++ the tightest coupling is Friend, second is Inheritance,
-      then Containment and last is Usage through reference, pointer or handle.
-    <H3>Details <input type="button" value="Hide" onclick="toggleVisibility( this, 'coupling_details' );"/></H3>
-    <ARTICLE class="detail" id="coupling_details">
-      <UL>
-        <LI>Friend defines a "must-know" about details of implementation, don't use it unless your happy stating that Xxx really <B>must</B> know about Yyy implementation. and Yyy can never change without informing Xxx.
-        <LI>Inheritance defines a "is-a" relationship, don't use it unless you really can naturally say Xxx is-a Yyy. Most of the cases containment through interface is what you
-        <LI>Containment defines a "owns-a" relationship, use it when you have a natural Xxx owns-a Yyy relationship.
-      </UL>
-      Most of the time containment through interface and normal usage is what you should go for.
-      Strong ownership always beats sharing through reference counting. Reference counting means "part owns".
-      You would not want to part own anything in real life, so why do that in software? sooner or later it will leak,
-    </ARTICLE>
-    </P>
-
-  Two key principles to follow:
-  <H2>Open Closed Principle</H2>
-    <P>
-      Software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification.
-      That is, such an entity can allow its behaviour to be modified without altering its source code. Techniqu
-    </P>
-
-  <H2>Dependency Inversion Principle</H2>
-    <P>
-      High-level modules should not depend on low-level modules. Both should depend on abstractions.
-      Abstractions should not depend upon details. Details should depend upon abstractions.
-    </P>
-
-<H1>General conventions</H1>
+<H1>General coding conventions</H1>
    <H2>Type casting <input type="button" value="Hide" onclick="toggleVisibility( this, 'ccasts_details' );"/></H2>
      <ARTICLE class="detail" id="ccasts_details">
        <SUMMARY>
    <H2>Type casting <input type="button" value="Hide" onclick="toggleVisibility( this, 'ccasts_details' );"/></H2>
      <ARTICLE class="detail" id="ccasts_details">
        <SUMMARY>
@@ -145,10 +55,10 @@ function toggleVisibility( button, obj )
          <B>Avoid using pointer or reference casts. They have been referred to as the goto of OO programming, fix the design instead</B><BR>
        </SUMMARY>
        <CODE class="good">
          <B>Avoid using pointer or reference casts. They have been referred to as the goto of OO programming, fix the design instead</B><BR>
        </SUMMARY>
        <CODE class="good">
-        X* ptr = static_cast<X*>( y_ptr ); // ok, compiler checks whether types are compatible
+  X* ptr = static_cast&#x3C;X*&#x3E;( y_ptr ); // ok, compiler checks whether types are compatible
        </CODE>
        <CODE class="bad">
        </CODE>
        <CODE class="bad">
-        (Foo*) ptr; // bad! C-cast is not guaranteed to check and never complains
+  (Foo*) ptr; // bad! C-cast is not guaranteed to check and never complains
        </CODE>
      </ARTICLE>
  
        </CODE>
      </ARTICLE>
  
@@ -174,13 +84,21 @@ function toggleVisibility( button, obj )
          This could lead to unexpected results. The same goes with default constructor and copy constructor.
        </P>
        <CODE class="good">
          This could lead to unexpected results. The same goes with default constructor and copy constructor.
        </P>
        <CODE class="good">
-        class X
-        {
-          X();                      // default constructor
-          X( const X& );            // copy constructor
-          X& operator=( const X& ); // copy assignment operator
-          ~X();                     // destructor
-        };
+  class X
+  {
+    X();                      // default constructor
+    X( const X& );            // copy constructor
+    X& operator=( const X& ); // copy assignment operator
+    ~X();                     // destructor
+  };
+
+  class X
+  {
+    X();                                // default constructor
+    ~X();                               // destructor
+    X( const X& ) = delete;             // copy constructor not allowed
+    X& operator=( const X& ); = delete; // copy assignment operator not allowed
+  };
        </CODE>
      </ARTICLE>
    <H2>Class types <input type="button" value="Hide" onclick="toggleVisibility( this, 'class_types_details' );"/></H2>
        </CODE>
      </ARTICLE>
    <H2>Class types <input type="button" value="Hide" onclick="toggleVisibility( this, 'class_types_details' );"/></H2>
@@ -197,7 +115,7 @@ function toggleVisibility( button, obj )
        <P>
          Classes with <B>Pointer</B> semantics are always passed through pointers, references or smart pointers.
          These classes are ususally compound types that cannot be easily copied and thus prevent copy constructor,
        <P>
          Classes with <B>Pointer</B> semantics are always passed through pointers, references or smart pointers.
          These classes are ususally compound types that cannot be easily copied and thus prevent copy constructor,
-        and assignment operator. That can be only stored on STL containers through smart pointers.
+        and assignment operator. They can be only stored on STL containers through smart pointers.
        </P>
      </ARTICLE>
    <H2>Access Rights <input type="button" value="Hide" onclick="toggleVisibility( this, 'access_details' );"/></H2>
        </P>
      </ARTICLE>
    <H2>Access Rights <input type="button" value="Hide" onclick="toggleVisibility( this, 'access_details' );"/></H2>
@@ -235,21 +153,21 @@ function toggleVisibility( button, obj )
        <P>
          Declare all single argument constructors as explicit thus preventing their use as implicit type convertors.
          <CODE class="good">
        <P>
          Declare all single argument constructors as explicit thus preventing their use as implicit type convertors.
          <CODE class="good">
-          class C
-          {
-          public:
-            explicit C( int );         // good, explicit
-            C( int, int );             // ok more than one non-default argument
-          };
+  class C
+  {
+  public:
+    explicit C( int );         // good, explicit
+    C( int, int );             // ok more than one non-default argument
+  };
          </CODE>
          <CODE class="bad">
          </CODE>
          <CODE class="bad">
-          class C
-          {
-          public:
-            C( double );               // bad, can be used in implicit conversion
-            C( float f, int i=0 );     // bad, implicit conversion constructor
-            C( int i=0, float f=0.0 ); // bad, default constructor, but also a conversion constructor
-          };
+  class C
+  {
+  public:
+    C( double );               // bad, can be used in implicit conversion
+    C( float f, int i=0 );     // bad, implicit conversion constructor
+    C( int i=0, float f=0.0 ); // bad, default constructor, but also a conversion constructor
+  };
          </CODE>
        </P>
      </ARTICLE>
          </CODE>
        </P>
      </ARTICLE>
@@ -281,11 +199,11 @@ function toggleVisibility( button, obj )
        <P>
          Don't shortcut, like use the returned reference of getter to assign a new value. If a Setter is missing, add it!
          <CODE class="bad">
        <P>
          Don't shortcut, like use the returned reference of getter to assign a new value. If a Setter is missing, add it!
          <CODE class="bad">
-            initial.GetPosition() = Position(10, 10); // bad!, If GetPosition is one day changed to return copy
-                                                      // of Position this code silently changes to a no-op.
+  initial.GetPosition() = Position(10, 10); // bad!, If GetPosition is one day changed to return copy
+                                            // of Position this code silently changes to a no-op.
          </CODE>
          <CODE class="good">
          </CODE>
          <CODE class="good">
-            initial.SetPosition( Position( 10, 10 ) );
+  initial.SetPosition( Position( 10, 10 ) );
          </CODE>
        </P>
        <P>
          </CODE>
        </P>
        <P>
@@ -303,29 +221,243 @@ function toggleVisibility( button, obj )
  
          GCC automatically inlines member functions defined within the class body of C++ programs even if they are not explicitly declared inline.
          <CODE class="bad">
  
          GCC automatically inlines member functions defined within the class body of C++ programs even if they are not explicitly declared inline.
          <CODE class="bad">
-               class Color
-               {
-                       inline float& GetRed()  { return mRed;   }              // inline keyword not needed
-                       inline float& GetGreen(){ return mGreen; }
-               };
+  class Color
+  {
+    inline float& GetRed()  { return mRed;   } // inline keyword not needed
+    inline float& GetGreen(){ return mGreen; }
+  };
+        </CODE>
+        <CODE class="good">
+  class Color
+  {
+    float& GetRed()   { return mRed;   }
+    float& GetGreen(){ return mGreen; }
+  };
+        </CODE>
+      </P>
+      <P>
+        If there are a lot of inlines, they should be in a .inl file.
+        Remember the inline keyword is just a hint to the compiler. Whether
+        a function will be inlined or not is down to the compiler and its flags.
+      </P>
+      <P>
+      </P>
+    </ARTICLE>
+
+  <H2>Conversion operators <input type="button" value="Hide" onclick="toggleVisibility( this, 'conversion_details' );"/></H2>
+    <ARTICLE class="detail" id="conversion_details">
+      <P>
+
+        Don't declare implicit conversion operators in classes. They allow the compiler to trip you up and go from one type to another via the conversion operator unintentionally.
+        Conversion operators are particularly dangerous in conjunction with auto keyword. If conversion is required, make it explicit or better yet, add a getter with a more meaningfull name.
+        <CODE class="bad">
+
+  // Bad:
+  class SmallInt
+  {
+  public:
+    // implicit conversion to float
+    operator float() const { return float( val ); }
+  private:
+    int val;
+  };
+  //... and in the program:
+
+  int main( void )
+  {
+    int value;
+    SmallValue foo;
+    value = foo; // oops, didn't really want to allow conversion to int but the compiler can do that as float can be assigned to int.
+
+    return 0;
+  }
          </CODE>
          <CODE class="good">
          </CODE>
          <CODE class="good">
-               class Color
-               {
-                       float& GetRed()   { return mRed;   }
-                       float& GetGreen(){ return mGreen; }
-               };
+
+  // Good:
+  class SmallInt
+  {
+  public:
+    // explicit getter for float
+    float AsFloat const { return static_cast&#x3C;float&#x3E;( val ); }
+  private:
+    int val;
+  };
+  //... and in the program:
+
+  int main( void )
+  {
+    int value;
+    SmallValue foo;
+    si.AsFloat() + 3; // ok: explicitly request the conversion
+
+    return 0;
+  }
+
+  // Good:
+  class SmallInt
+  {
+  public:
+    // explicit conversion to int
+    explicit operator int() const { return val; }
+  private:
+    int val;
+  };
+  //... and in the program:
+
+  int main()
+  {
+    SmallInt si = 3; // ok: the SmallInt constructor is not explicit
+    si + 3; // error: implicit is conversion required, but operator int is explicit
+    static_cast&#x3C;int&#x3E;(si) + 3; // ok: explicitly request the conversion
+
+    return 0;
+  }
+
          </CODE>
        </P>
        <P>
          </CODE>
        </P>
        <P>
-       If there are a lot of inlines, they should be in a .inl file.
-       Remember the inline keyword is just a hint to the compiler. Whether
-       a function will be inlined or not is down to the compiler and its flags.
+      </P>
+    </ARTICLE>
+
+  <H2>Auto keyword <input type="button" value="Hide" onclick="toggleVisibility( this, 'auto_details' );"/></H2>
+    <ARTICLE class="detail" id="auto_details">
+      <P>
+
+        auto keyword should only be used where it improves the readability of the code and does not lead to ambiguities.
+        never use auto in a line where multiple different types occur as part of expressions like additions, subtracts, multiplies as whe conversion ordering rules are not always obvious.
+
+        <CODE class="good">
+
+  // Good:
+  auto actor = Actor::DownCast(GetOwner()); // it is obvious that actor is of type Actor so no need to retype the type
+  auto widthMode = widthMeasureSpec.GetMode(); // it is relatively obvious that Mode is an enumeration with potentially long name so no need to repeat the type, no ambiguity
+  auto childLayout = GetChildAt( i ); // name of the variable is clear enough indication of the type, no ambiguity
+  auto childPosition = childOwner.GetProperty&#x3C; Dali::Vector3 &#x3E;( Actor::Property::POSITION ); // getter already contains the type, no need to repeat it
+
+  for ( auto&& renderTask : mImpl->taskList.GetTasks() ) // iterator type not relevant for the algorithm, code much cleaner with auto
+  {
+    renderTask->UpdateState();
+  }
+
+        </CODE>
+        <CODE class="bad">
+
+  // Bad:
+  auto width = layout->GetWidth() - padding.end - padding.start; // not obvious what the final type ends up as multiple type conversions may occur
+
+  auto size = std::max( LayoutLength(0), specSize - padding ); // not obvious which of the types is preferred by compiler; or what the type of specSize - padding actually is
+
+  auto minPosition = Vector3( Vector3::ZERO ); // auto does not add any value here
+        </CODE>
+        <CODE class="good">
+  // Good:
+  Vector3 minPosition; // vector initializes to 0,0,0
+        </CODE>
+        <CODE class="bad">
+  // Bad:
+  auto specification = MeasureSpec( GetMeasuredHeight(), MeasureSpec::Mode::EXACTLY ); // no value in typing auto in assignment, much cleaner and less ambiguous to write:
+        </CODE>
+        <CODE class="good">
+  // Good:
+  MeasureSpec specification( GetMeasuredHeight(), MeasureSpec::Mode::EXACTLY ); // obvious construction of a type with parameters
+        </CODE>
+
        </P>
        <P>
        </P>
      </ARTICLE>
  
        </P>
        <P>
        </P>
      </ARTICLE>
  
+<H1>General design priciples</H1>
+  <P>
+    Here's a few pragmatic programmer guidelines to follow (<A HREF="http://www.codinghorror.com/blog/files/Pragmatic%20Quick%20Reference.htm">Web version</A>)
+    <H3>Details <input type="button" value="Hide" onclick="toggleVisibility( this, 'general_details' );"/></H3>
+    <ARTICLE class="detail" id="general_details">
+      <UL>
+      <LI><B>Care About the Software, Care about your API users and end users</B><BR>
+            Why spend your life developing software unless you care about doing it well?
+            Turn off the autopilot and take control. Constantly critique and appraise your work.
+      <LI><B>Don't Live with Broken Windows</B><BR>
+            Fix bad designs, wrong decisions, and poor code when you see them.
+            You can't force change on people. Instead, show them how the future might be and help them participate in creating it.
+      <LI><B>Remember the Big Picture</B><BR>
+            Don't get so engrossed in the details that you forget to check what's happening around you.
+      <LI><B>DRY - Don't Repeat Yourself</B><BR>
+            Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.
+      <LI><B>Eliminate Effects Between Unrelated Things</B><BR>
+            Design components that are self-contained. independent, and have a single, well-defined purpose.
+      <LI><B>There Are No Final Decisions</B><BR>
+            No decision is cast in stone. Instead, consider each as being written in the sand at the beach, and plan for change.
+      <LI><B>Fix the Problem, Not the Blame</B><BR>
+            It doesn't really matter whether the bug is your fault or someone else's—it is still your problem, and it still needs to be fixed.
+      <LI><B>You Can't Write Perfect Software</B><BR>
+            Software can't be perfect. Protect your code and users from the inevitable errors.
+      <LI><B>Design with Contracts</B><BR>
+            Use contracts to document and verify that code does no more and no less than it claims to do.
+      <LI><B>Crash Early</B><BR>
+            A dead program normally does a lot less damage than a crippled one.
+      <LI><B>Use Assertions to Prevent the Impossible</B><BR>
+            Assertions validate your assumptions. Use them to protect your code from an uncertain world.
+      <LI><B>Use Exceptions for Exceptional Problems</B><BR>
+            Exceptions can suffer from all the readability and maintainability problems of classic spaghetti code. Reserve exceptions for exceptional things.
+      <LI><B>Minimize Coupling Between Modules</B><BR>
+            Avoid coupling by writing "shy" code and applying the Law of Demeter.
+      <LI><B>Put Abstractions in Code, Details in Metadata</B><BR>
+            Program for the general case, and put the specifics outside the compiled code base.
+      <LI><B>Always Design for Concurrency</B><BR>
+            Allow for concurrency, and you'll design cleaner interfaces with fewer assumptions.
+      <LI><B>Don't Program by Coincidence</B><BR>
+            Rely only on reliable things. Beware of accidental complexity, and don't confuse a happy coincidence with a purposeful plan.
+      <LI><B>Test Your Estimates</B><BR>
+            Mathematical analysis of algorithms doesn't tell you everything. Try timing your code in its target environment.
+      <LI><B>Refactor Early, Refactor Often</B><BR>
+            Just as you might weed and rearrange a garden, rewrite, rework, and re-architect code when it needs it. Fix the root of the problem.
+      <LI><B>Design to Test</B><BR>
+            Start thinking about testing before you write a line of code.
+      <LI><B>Abstractions Live Longer than Details</B><BR>
+            Invest in the abstraction, not the implementation. Abstractions can survive the barrage of changes from different implementations and new technologies.
+      <LI><B>Coding Ain't Done 'Til All the Tests Run</B><BR>
+            'Nuff said.
+      <LI><B>Use Saboteurs to Test Your Testing</B><BR>
+            Introduce bugs on purpose in a separate copy of the source to verify that testing will catch them.
+      <LI><B>Find Bugs Once</B><BR>
+            Once a human tester finds a bug, it should be the last time a human tester finds that bug. Automatic tests should check for it from then on.
+      <LI><B>Sign Your Work</B><BR>
+            Craftsmen of an earlier age were proud to sign their work. You should be, too.
+      </UL>
+    </ARTICLE>
+  </P>
+
+  <H2>Avoid Tight Coupling</H2>
+    <P>
+      Always choose the loosest possible coupling between entities. In C++ the tightest coupling is Friend, second is Inheritance,
+      then Containment and last is Usage through reference, pointer or handle.
+    <H3>Details <input type="button" value="Hide" onclick="toggleVisibility( this, 'coupling_details' );"/></H3>
+    <ARTICLE class="detail" id="coupling_details">
+      <UL>
+        <LI>Friend defines a "must-know" about details of implementation, don't use it unless your happy stating that Xxx really <B>must</B> know about Yyy implementation. and Yyy can never change without informing Xxx.
+        <LI>Inheritance defines a "is-a" relationship, don't use it unless you really can naturally say Xxx is-a Yyy. Most of the cases containment through interface is what you
+        <LI>Containment defines a "owns-a" relationship, use it when you have a natural Xxx owns-a Yyy relationship.
+      </UL>
+      Most of the time containment through interface and normal usage is what you should go for.
+      Strong ownership always beats sharing through reference counting. Reference counting means "part owns".
+      You would not want to part own anything in real life, so why do that in software? sooner or later it will leak,
+    </ARTICLE>
+    </P>
+
+  Two key principles to follow:
+  <H2>Open Closed Principle</H2>
+    <P>
+      Software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification.
+      That is, such an entity can allow its behaviour to be modified without altering its source code. Techniqu
+    </P>
+
+  <H2>Dependency Inversion Principle</H2>
+    <P>
+      High-level modules should not depend on low-level modules. Both should depend on abstractions.
+      Abstractions should not depend upon details. Details should depend upon abstractions.
+    </P>
+
    <P>Thats all folks, if you read this far you are now all equipped to write good code :) !! </P>
  
  </BODY>
    <P>Thats all folks, if you read this far you are now all equipped to write good code :) !! </P>
  
  </BODY>
author	Kimmo Hoikka <kimmo.hoikka@samsung.com>
	Fri, 14 Sep 2018 16:55:29 +0000 (17:55 +0100)
committer	Kimmo Hoikka <kimmo.hoikka@samsung.com>
	Fri, 14 Sep 2018 17:08:51 +0000 (18:08 +0100)