From: Kimmo Hoikka Date: Fri, 14 Sep 2018 16:55:29 +0000 (+0100) Subject: Update Coding convention regarding conversion operators and use of auto keyword X-Git-Tag: dali_1.3.42~3 X-Git-Url: http://review.tizen.org/git/?p=platform%2Fcore%2Fuifw%2Fdali-core.git;a=commitdiff_plain;h=88056efee02562ebe99e72842d45cc919e7dc5c1 Update Coding convention regarding conversion operators and use of auto keyword Change-Id: I24432c9af972f87638114cf49de342371a81e6a0 --- diff --git a/docs/coding-convention.html b/docs/coding-convention.html index 79bbf03..1acc2f1 100644 --- a/docs/coding-convention.html +++ b/docs/coding-convention.html @@ -41,97 +41,7 @@ function toggleVisibility( button, obj ) -

General

-

- Here's a few pragmatic programmer guidelines to follow (Web version) -

Details

-
- -
-

- -

Avoid Tight Coupling

-

- 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. -

Details

-
- - 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, -
-

- - Two key principles to follow: -

Open Closed Principle

-

- 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 -

- -

Dependency Inversion Principle

-

- 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. -

- -

General conventions

+

General coding conventions

Type casting

@@ -145,10 +55,10 @@ function toggleVisibility( button, obj ) Avoid using pointer or reference casts. They have been referred to as the goto of OO programming, fix the design instead
 - X* ptr = static_cast( y_ptr ); // ok, compiler checks whether types are compatible + X* ptr = static_cast<X*>( y_ptr ); // ok, compiler checks whether types are compatible - (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
@@ -174,13 +84,21 @@ function toggleVisibility( button, obj ) This could lead to unexpected results. The same goes with default constructor and copy constructor.

- 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 + };

Class types

@@ -197,7 +115,7 @@ function toggleVisibility( button, obj )

Classes with Pointer 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.

Access Rights

@@ -235,21 +153,21 @@ function toggleVisibility( button, obj )

Declare all single argument constructors as explicit thus preventing their use as implicit type convertors. - 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 + }; - 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 + };

@@ -281,11 +199,11 @@ function toggleVisibility( button, obj )

Don't shortcut, like use the returned reference of getter to assign a new value. If a Setter is missing, add it! - 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. - initial.SetPosition( Position( 10, 10 ) ); + initial.SetPosition( Position( 10, 10 ) );

@@ -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. - 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; } + }; + + + class Color + { + float& GetRed() { return mRed; } + float& GetGreen(){ return mGreen; } + }; + +

+

+ 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. +

+

+

+ + +

Conversion operators

+
+

+ + 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. + + + // 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; + } - class Color - { - float& GetRed() { return mRed; } - float& GetGreen(){ return mGreen; } - }; + + // Good: + class SmallInt + { + public: + // explicit getter for float + float AsFloat const { return static_cast<float>( 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<int>(si) + 3; // ok: explicitly request the conversion + + return 0; + } +

- 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. +

+
+ +

Auto keyword

+
+

+ + 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. + + + + // 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< Dali::Vector3 >( 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(); + } + + + + + // 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 + + + // Good: + Vector3 minPosition; // vector initializes to 0,0,0 + + + // Bad: + auto specification = MeasureSpec( GetMeasuredHeight(), MeasureSpec::Mode::EXACTLY ); // no value in typing auto in assignment, much cleaner and less ambiguous to write: + + + // Good: + MeasureSpec specification( GetMeasuredHeight(), MeasureSpec::Mode::EXACTLY ); // obvious construction of a type with parameters + +

+

General design priciples

+

+ Here's a few pragmatic programmer guidelines to follow (Web version) +

Details

+
+ +
+

+ +

Avoid Tight Coupling

+

+ 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. +

Details

+
+ + 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, +
+

+ + Two key principles to follow: +

Open Closed Principle

+

+ 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 +

+ +

Dependency Inversion Principle

+

+ 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. +

+

Thats all folks, if you read this far you are now all equipped to write good code :) !!