Skip to content

Instantly share code, notes, and snippets.

@BenjamenMeyer

BenjamenMeyer/gist:a3454e4dce1787af30e60f7180b650d1

Created July 15, 2021 03:43
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save BenjamenMeyer/a3454e4dce1787af30e60f7180b650d1 to your computer and use it in GitHub Desktop.
Save BenjamenMeyer/a3454e4dce1787af30e60f7180b650d1 to your computer and use it in GitHub Desktop.
Download ZIP
Review of ISOCPP Guidelines
Raw
gistfile1.txt
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#abstract
"By “modern C++” we mean effective use of the ISO C++ standard (currently C++17, but almost all of our recommendations also apply to C++14 and C++11)"
IOW, it's primarily written against a version of C++ that we already know we can't use because not all platforms support C++17.
At best, our common denominator is C++14 but most likely only C++11. I wouldn't go beyond C++11 at this time.
============================================================================================================================================================================================================
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#p1-express-ideas-directly-in-code
While I agree in concept; their example is bad.
1. Using std::find() should be called out as `std::find` not simply `find()` as it makes for bad namespace integration.
2. Assumes the underlying data being searched can be searched with `std::find()`; in the specific example they're using `std::vector` (again it should be `std::vector` not simply `vector`).
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#p2-write-in-iso-standard-c
I'll agree here with the caveat that not all language features in C++11 and later are useful, necessary, or make the code more readable.
In fact I find a lot of things the ISO C++ Standards committee does to be the opposite of that.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#p3-express-intent
This is a must; however, their use of `auto` in bad.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#p4-ideally-a-program-should-be-statically-type-safe
Agree in principle; but don't agree with their examples.
Templates (and Generics) often make things more complex and harder that solid types. Yes there's a place for them but no where near as much as the ISO C++ committee would have you believe.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#p5-prefer-compile-time-checking-to-run-time-checking
The problem here is one of Secure Software Development Practices.
Certainly things need to be checked as much as possible at compile time; however, that doesn't negate the need for dynamic checking at run-time to validate data is what is expected.
Also, `assert` should pretty much never be used.
The compiler will do things that are unintentional and lead to bad security postures. Don't necessarily trust the compiler to do the right thing because it very well may not be the right thing.
Their `read` example here exhibits this bad behavior potential as well.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#p6-what-cannot-be-checked-at-compile-time-should-be-checkable-at-run-time
Their example here is one of an architectural failure. Data that is allocated should be deallocated only by the same mechanism that allocated it.
If an object (struct, class, etc) uses one means of allocation, then it should provide a means to do so and an equivalent means of deallocating it.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#p7-catch-run-time-errors-early
Agreed; though their examples are bad.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#p8-dont-leak-any-resources
Agreed; though single-entry single-exit practices also help mitigate this dramatically.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#p10-prefer-immutable-data-to-mutable-data
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#p11-encapsulate-messy-constructs-rather-than-spreading-through-the-code
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#p12-use-supporting-tools-as-appropriate
Agreed
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#p13-use-support-libraries-as-appropriate
Agreed; though it should be obvious that is the case.
For example, don't use the `using namespace` keywords outside of the shortening the local namespace.
- header files should explicitly list all namespace; `using namespace` must be avoided in headers
- implementation files:
- all external namespaces should be left alone and directly referenced.
- `using namespace` should be limited to what the implementation file is implementing in so there should only be a single `using namespace` declaration.
For example, the implementation file is working in the `vegastrike::foo` namespace; only `using namespace vegastrike::foo` is allowed; any other namespace (however long) should be directly referenced.
============================================================================================================================================================================================================
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i1-make-interfaces-explicit
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i10-use-exceptions-to-signal-a-failure-to-perform-a-required-task
Agreed; however, in general Exceptions are bad - namely because programmers tend to ignore them until they must explicitly handle them which means errors don't get caught early or where they can be fixed.
However, since we're integrating with Python where Exceptions are the norm, we must use Exceptions. That's not to say a global `errno` should be used - it shouldn't; but a better design is to have object specific error states that is exposed through the API definition.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i2-avoid-non-const-global-variables
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i4-make-interfaces-precisely-and-strongly-typed
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i12-declare-a-pointer-that-must-not-be-null-as-not_null
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i24-avoid-adjacent-parameters-that-can-be-invoked-by-the-same-arguments-in-either-order-with-different-meaning
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i25-prefer-empty-abstract-classes-as-interfaces-to-class-hierarchies
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i30-encapsulate-rule-violations
Agreed
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i3-avoid-singletons
Agreed; though the one exception to the rule here would be our configuration object. The race condition can be resolved by how it's explicitly initialized - we don't rely on C++ to determine when it's initialized but do so as part of the program operation explicitly.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i5-state-preconditions-if-any
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i6-prefer-expects-for-expressing-preconditions
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i7-state-postconditions
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i8-prefer-ensures-for-expressing-postconditions
This is best done through documentation. However, avoid `Expects()` and avoid assertions. Generate a catchable error instead.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i9-if-an-interface-is-a-template-document-its-parameters-using-concepts
Ehh...not sure; doesn't matter as we can't do C++20 right now any way, but in general avoid Templates so this doesn't matter from that aspect too.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i11-never-transfer-ownership-by-a-raw-pointer-t-or-reference-t
Ownership must be clear from an architectural design. Honestly I like Qt's parent:child relationship mechanism for that best since the creator is the parent and everyone else can just toss it around.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i13-do-not-pass-an-array-as-a-single-pointer
Mixed on this one.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i22-avoid-complex-initialization-of-global-objects
Agreed - initialization of global objects should be handled early on and in specific order. It should be very explicit when and where it happens.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i23-keep-the-number-of-function-arguments-low
Agreed - if they're getting to be too many then you probably need a struct to pass the data in.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i26-if-you-want-a-cross-compiler-abi-use-a-c-style-subset
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i27-for-stable-library-abi-consider-the-pimpl-idiom
Agreed. PIMPL isn't an easy thing to work with though; however, our primary ABI is going to be the Python API side so this probably doesn't matter as much.
============================================================================================================================================================================================================
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f1-package-meaningful-operations-as-carefully-named-functions
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f2-a-function-should-perform-a-single-logical-operation
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f5-if-a-function-is-very-small-and-time-critical-declare-it-inline
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f6-if-your-function-must-not-throw-declare-it-noexcept
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f15-prefer-simple-and-conventional-ways-of-passing-information
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f16-for-in-parameters-pass-cheaply-copied-types-by-value-and-others-by-reference-to-const
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f17-for-in-out-parameters-pass-by-reference-to-non-const
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f20-for-out-output-values-prefer-return-values-to-output-parameters
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f21-to-return-multiple-out-values-prefer-returning-a-struct-or-tuple
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f42-return-a-t-to-indicate-a-position-only
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f43-never-directly-or-indirectly-return-a-pointer-or-a-reference-to-a-local-object
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f44-return-a-t-when-copy-is-undesirable-and-returning-no-object-isnt-needed
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f45-dont-return-a-t
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f46-int-is-the-return-type-for-main
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f47-return-t-from-assignment-operators
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f48-dont-return-stdmovelocal
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f51-where-there-is-a-choice-prefer-default-arguments-over-overloading
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f54-if-you-capture-this-capture-all-variables-explicitly-no-default-capture
Agreed
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f3-keep-functions-short-and-simple
Agreed. Long functions should be the exception to the rule. This is generally easily enforced.
Shorter functions are also easier to unit test.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f4-if-a-function-might-have-to-be-evaluated-at-compile-time-declare-it-constexpr
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f18-for-will-move-from-parameters-pass-by-x-and-stdmove-the-parameter
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f19-for-forward-parameters-pass-by-tp-and-only-stdforward-the-parameter
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f24-use-a-spant-or-a-span_pt-to-designate-a-half-open-sequence
Not sure here.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f7-for-general-use-take-t-or-t-arguments-rather-than-smart-pointers
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f22-use-t-or-ownert-to-designate-a-single-object
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f60-prefer-t-over-t-when-no-argument-is-a-valid-option
Disagree here. The correct smart pointer can transfer reference to the object appropriately without transferring ownership, and templates should be avoided.
Smart pointers also offer the capability to check the validity of the pointer (secure programming practices) using defined interfaces over the raw pointer.
Obviously if you can pass by reference instead of by pointer that is much preferred since it eliminates the whole issue to start with. But in complex software that is highly unlikely.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f8-prefer-pure-functions
Disagree. Templates should be avoided when at all possible.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f9-unused-parameters-should-be-unnamed
Agreed; however I'd go further to state that the name should be commented out:
X* find(std::map<Blob>& m, const string& s, Hint /* h */ ); // once upon a time, a hint was used that was named `h`
This allowed for it to be easily added back in, and tracked over time.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f23-use-a-not_nullt-to-indicate-that-null-is-not-a-valid-value
This is unnecessary because per Secure Development Practices you (a) shouldn't trust input and (b) shouldn't trust output so it should be checked on input and by the caller on output.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f25-use-a-zstring-or-a-not_nullzstring-to-designate-a-c-style-string
Disagree namely b/c one should move to an std::string or std::wstring ASAP.
At least Qt introduced the ability of their QString to be able to reference single instances of RAW C Strings without copying the data. I don't see this capability in the C++ standards yet sadly, only a version that does the move semantics.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f26-use-a-unique_ptrt-to-transfer-ownership-where-a-pointer-is-needed
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f27-use-a-shared_ptrt-to-share-ownership
Considering how many times they've swapped around the smart pointer classes it's kind of hard to follow.
Generally I'd like to agree here; but they've also made a little mess of it.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f50-use-a-lambda-when-a-function-wont-do-to-capture-local-variables-or-to-write-a-local-function
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f52-prefer-capturing-by-reference-in-lambdas-that-will-be-used-locally-including-passed-to-algorithms
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f53-avoid-capturing-by-reference-in-lambdas-that-will-be-used-non-locally-including-returned-stored-on-the-heap-or-passed-to-another-thread
Absolutely disagree. Lambdas in C++ should be avoided at nearly all costs.
There is a rare exception for them, but it's extremely rare, especially when not using signal/slot mechanics (e.g Qt, Boost:Signal) where the lambda is only useful for extremely simple connections:
Qt::Connect(SomeObj, SomeSignal, MyObj, lambda: []{ MyObj.NonSlottableFunction(); })
In general, Lambdas lead to bad code, bad style that is hard to debug, hard to locate, and nearly impossible to test.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f55-dont-use-va_arg-arguments
Disagree. There are good use-cases for Variadic Arguments. That said, they should be the exception not the rule.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f56-avoid-unnecessary-condition-nesting
Disagree; this is a good pattern for single-exit-single-entry implementation while preserving input validation.
============================================================================================================================================================================================================
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c1-organize-related-data-into-structures-structs-or-classes
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c3-represent-the-distinction-between-an-interface-and-an-implementation-using-a-class
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c5-place-helper-functions-in-the-same-namespace-as-the-class-they-support
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c7-dont-define-a-class-or-enum-and-declare-a-variable-of-its-type-in-the-same-statement
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c9-minimize-exposure-of-members
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cconcrete-concrete-types
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c11-make-concrete-types-regular
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c22-make-default-operations-consistent
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c31-all-resources-acquired-by-a-class-must-be-released-by-the-classs-destructor
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c33-if-a-class-has-an-owning-pointer-member-define-a-destructor
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c41-a-constructor-should-create-a-fully-initialized-object
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c44-prefer-default-constructors-to-be-simple-and-non-throwing
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c47-define-and-initialize-member-variables-in-the-order-of-member-declaration
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c50-use-a-factory-function-if-you-need-virtual-behavior-during-initialization
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c52-use-inheriting-constructors-to-import-constructors-into-a-derived-class-that-does-not-need-further-explicit-initialization
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c61-a-copy-operation-should-copy
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c62-make-copy-assignment-safe-for-self-assignment
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c64-a-move-operation-should-move-and-leave-its-source-in-a-valid-state
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c65-make-move-assignment-safe-for-self-assignment
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c66-make-move-operations-noexcept
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c81-use-delete-when-you-want-to-disable-default-behavior-without-wanting-an-alternative
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c83-for-value-like-types-consider-providing-a-noexcept-swap-function
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c84-a-swap-function-must-not-fail
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c85-make-swap-noexcept
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c86-make--symmetric-with-respect-to-operand-types-and-noexcept
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c87-beware-of--on-base-classes
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c89-make-a-hash-noexcept
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c109-if-a-resource-handle-has-pointer-semantics-provide--and--
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c120-use-class-hierarchies-to-represent-concepts-with-inherent-hierarchical-structure-only
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c121-if-a-base-class-is-used-as-an-interface-make-it-a-pure-abstract-class
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c122-use-abstract-classes-as-interfaces-when-complete-separation-of-interface-and-implementation-is-needed
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c148-use-dynamic_cast-to-a-pointer-type-when-failure-to-find-the-required-class-is-considered-a-valid-alternative
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c153-prefer-virtual-function-to-casting
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c160-define-operators-primarily-to-mimic-conventional-usage
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c162-overload-operations-that-are-roughly-equivalent
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c163-overload-only-for-operations-that-are-roughly-equivalent
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c164-avoid-implicit-conversion-operators
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c168-define-overloaded-operators-in-the-namespace-of-their-operands
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c180-use-unions-to-save-memory
Agreed.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c46-by-default-declare-single-argument-constructors-explicit
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c145-access-polymorphic-objects-through-pointers-and-references
Agree... I think? It's not very clear.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c2-use-class-if-the-class-has-an-invariant-use-struct-if-the-data-members-can-vary-independently
Disagree - at least with their presentation.
Classes should be used when the data needs to be protected to make sure it's properly processed.
Structs should be used when the data doesn't need to be protected but needs to be associated for various reasons.
In general, Classes are the appropriate tool for doing OO.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c8-use-class-rather-than-struct-if-any-member-is-non-public
Agreed - but if using a Class than *all* members must be non-public.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c21-if-you-define-or-delete-any-copy-move-or-destructor-function-define-or-delete-them-all
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c60-make-copy-assignment-non-virtual-take-the-parameter-by-const-and-return-by-non-const
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c63-make-move-assignment-non-virtual-take-the-parameter-by--and-return-by-non-const
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c102-give-a-container-move-operations
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c104-give-a-container-a-default-constructor-that-sets-it-to-empty
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c126-an-abstract-class-typically-doesnt-need-a-user-written-constructor
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c146-use-dynamic_cast-where-class-hierarchy-navigation-is-unavoidable
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c147-use-dynamic_cast-to-a-reference-type-when-failure-to-find-the-required-class-is-considered-an-error
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c150-use-make_unique-to-construct-objects-owned-by-unique_ptrs
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c151-use-make_shared-to-construct-objects-owned-by-shared_ptrs
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c166-overload-unary--only-as-part-of-a-system-of-smart-pointers-and-references
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c167-use-an-operator-for-an-operation-with-its-conventional-meaning
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c170-if-you-feel-like-overloading-a-lambda-use-a-generic-lambda
Fair enough
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c4-make-a-function-a-member-only-if-it-needs-direct-access-to-the-representation-of-a-class
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c10-prefer-concrete-types-over-class-hierarchies
Mixed on this.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c12-dont-make-data-members-const-or-references
Mixed - agree on references; but disagree on Constants. Constants internal to a class (protected, private) can be useful for scoping purposes.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c20-if-you-can-avoid-defining-default-operations-do
Disagree. Constructors nearly always are about more than simply initializing data; but even it is was just initializing data being explicit is better than implicit.
In this case, implicit means leaving it to the compiler. Being explicit gives you better control over debugging code.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c30-define-a-destructor-if-a-class-needs-an-explicit-action-at-object-destruction
Disagree. Always define the destructor. More often than not resources need to be cleaned up. Ignoring destructors is one of the most common means of leaking resources,
and the default destructors will not release memory for you, and smart pointers do not always protect for cleanup.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c32-if-a-class-has-a-raw-pointer-t-or-reference-t-consider-whether-it-might-be-owning
Agreed - By default, the class should own it. However, this is an architectural issue. Some frameworks (e.g Qt) define this for you (parent always cleans up its children); but mostly applications need to define the ownership themselves.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c35-a-base-class-destructor-should-be-either-public-and-virtual-or-protected-and-non-virtual
Agreed - caveat: unless there is explicit reason to otherwise, all destructors should be public and virtual.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c36-a-destructor-must-not-fail
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c37-make-destructors-noexcept
Agreed - errors must be handled by the destructor, it can't throw exceptions. Any errors here will be completely untraceable and be generated at seemingly random
points in code operation.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c40-define-a-constructor-if-a-class-has-an-invariant
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c43-ensure-that-a-copyable-value-type-class-has-a-default-constructor
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c80-use-default-if-you-have-to-be-explicit-about-using-the-default-semantics
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c127-a-class-with-a-virtual-function-should-have-a-virtual-or-protected-destructor
Moot b/c constructors and destructors should always be defined.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c42-if-a-constructor-cannot-construct-a-valid-object-throw-an-exception
Disagree - constructors should never throw Exceptions. again, this will lead to exceptions being thrown from seemingly random code locations.
A valid object should be the minimal valid object with options to be able to make it a fully valid object; the difference should be possible through member functions.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers
Disagree - one shouldn't use in-class initializers; constructor default parameters are better.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c49-prefer-initialization-to-assignment-in-constructors
Agreed though
class A {
private:
string s1;
public:
A(czstring p): s1(p) {}
}
Is better.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c51-use-delegating-constructors-to-represent-common-actions-for-all-constructors-of-a-class
Agreed - minimize the number of constructors
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c67-a-polymorphic-class-should-suppress-public-copymove
Agreed - and also why constructor/copy/move/destructor should be explicitly defined, not left to the compiler
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c82-dont-call-virtual-functions-in-constructors-and-destructors
Agreed - though constructors should always call the constructor of their parent class, and constructors should always be defined.
I've also found it good practice to do the same for destructors. So really this is moot because they should always be defined.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c90-rely-on-constructors-and-assignment-operators-not-memset-and-memcpy
For classes yes; for structs no. since structs are pure data this should be a problem. structs also define the C interface (separate from C++).
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c100-follow-the-stl-when-defining-a-container
Mixed - STL is useful in itself; but it's extremely hard to work with under a debugger, often requiring customized functions to the debugger to get useful stuff out of it (Qt isn't much different in some respects). It's hard to say what they mean based on the written text; so hard to say either way.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c101-give-a-container-value-semantics
Huh? Not sure they even know what they mean.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c103-give-a-container-an-initializer-list-constructor
Kinda...they should have constructors, so the constructor should take care of that.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c128-virtual-functions-should-specify-exactly-one-of-virtual-override-or-final
Once virtuals, always virtual.
`final` and `override` are unnecessary and shouldn't be used IMHO. Just keep it virtual and always make sure to declare any downstream implementation the same
as it's upstream partner.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c129-when-designing-a-class-hierarchy-distinguish-between-implementation-inheritance-and-interface-inheritance
While I agree about the interface vs implementation definition; I disagree regarding separating the implementation to explicit namespaces (e.g the dual hierarchy portion).
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c130-for-making-deep-copies-of-polymorphic-classes-prefer-a-virtual-clone-function-instead-of-public-copy-constructionassignment
Disagree since a copy constructor should start by calling the copy constructor on its parent in the hierarchy:
Class A {
A& operator=(A&);
}
class B: public A {
B& operator=(B&);
};
B& B::operator=(B& other) {
this = A::operator(other)
// continue with the specifics of copying B
return this;
}
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c131-avoid-trivial-getters-and-setters
Disagree. Getters/Setters are primarily used to hide the data and enforce using the interfaces provided. Classes should never have public data members only functional interfaces. Conversely structs should only have data and no functions (except perhaps a constructor for C++). If something outside the class needs access then a getter/setter should be provided as relevant - not every member needs a setter; but if there's a setter there definitely should be a getter.
Trivial versions can be implemented as inline methods, super trivial can be in the header itself too.
Verbosity is NOT a bad thing. Be explicit, not implicit. A class object is being used for a reason.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c132-dont-make-a-function-virtual-without-reason
Agreed - virtual carries overhead, so make sure it's necessary for downstream objects to be able to reimplement the function.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c133-avoid-protected-data
Disagree.
Personally, I always start a class definition with the template:
class A {
public:
A();
virtual ~A();
protected:
private:
}
I get that folks may want to force downstream objects to have to go through interfaces, but it really depends on the member variable.
It's hard to have a rule about what to allow/disallow here b/c it really is object dependant on what's good to do.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c134-ensure-all-non-const-data-members-have-the-same-access-level
Agreed with the exception of the above
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c135-use-multiple-inheritance-to-represent-multiple-distinct-interfaces
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c136-use-multiple-inheritance-to-represent-the-union-of-implementation-attributes
As a general rule multiple inheritance should be avoided if possible. It's not always possible, so when it must be used be clean about it, preferably inheriting only from pure abstract classes (interfaces).
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c137-use-virtual-bases-to-avoid-overly-general-base-classes
moot since multiple inheritance should be avoided
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c138-create-an-overload-set-for-a-derived-class-and-its-bases-with-using
Disagree.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c139-use-final-on-classes-sparingly
Agreed - though I'd say `final` should *never* be used
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c140-do-not-provide-different-default-arguments-for-a-virtual-function-and-an-overrider
While generally true, it really depends on what you're doing. Just make sure it's well documented or meaningful in the name:
class Square {
Square(uint8_t length);
}
class Square3 : public Square {
Square3(uint8_t length=3): Square(length) {}
}
class Square4 : public Square {
Square4(uint8_t length=4): Square(length) {}
}
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c149-use-unique_ptr-or-shared_ptr-to-avoid-forgetting-to-delete-objects-created-using-new
Agreed - though those aren't the only two smart pointer classes; but they also don't purely resolve memory leaks - they do help.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c152-never-assign-a-pointer-to-an-array-of-derived-class-objects-to-a-pointer-to-its-base
Agreed - though their example is extremely bad.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c161-use-non-member-functions-for-symmetric-operators
Symetric operations should be provably symetric; so this shouldn't matter. Generally, operators should be members of the class for clarity.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c165-use-using-for-customization-points
Disgree b/c one should be explicit. The namespace should be specified in all accounts.
For their example, if swap is static then it should be called using full specification: `N::swap(a, b)`; if it's an instance method then it should be called using one of the instances: `a.swap(b)`. If it's simply in the namespace then specify the namespace explicitly.
As a general rule, `using` should be avoided outside of the specific scope of what is being implemented - everything else should be explicitly referenced to their whole namespace.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c181-avoid-naked-unions
Huh??? Their definition here is kind of the definitive use of a `union`.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c182-use-anonymous-unions-to-implement-tagged-unions
Disagree - anonymous types (unions or otherwise) should be avoided. Just define them.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c183-dont-use-a-union-for-type-punning
Disagree with the caveat that using a union to type-pun should be the exception to the rule. In general yes it should be avoided.
However, in certain cases it makes great sense, like creating a variant type; in which case it should be paired with an enum to tell which portion of the union is valid.
============================================================================================================================================================================================================
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#enum1-prefer-enumerations-over-macros
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#enum2-use-enumerations-to-represent-sets-of-related-named-constants
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#enum4-define-operations-on-enumerations-for-safe-and-simple-use
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#enum6-avoid-unnamed-enumerations
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#enum7-specify-the-underlying-type-of-an-enumeration-only-when-necessary
Agreed
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#enum3-prefer-class-enums-over-plain-enums
Mixed - generally an enum should always be used as an integral value; however, class enums are useful for generating readable output (logs, etc).
Conversion functions for output are easy to do using a std::map:
enum class webColor {
red=0xFF0000,
green=0x00FF00,
blue=0x0000FF
};
std::string webColorToString(webColor wc) {
static bool initialized = false;
static std::map<webColor, std::string> values;
if !initialized {
// define a macro to add stuff to the map since the macro language
// makes it easy to define the string value
#define ADD_ENUM_TO_MAP(v) ( //
values[v] = std::string(#v); //
)
ADD_ENUM_TO_MAP(red)
ADD_ENUM_TO_MAP(blue)
ADD_ENUM_TO_MAP(green)
// clean up the macro namespace when done
#undef ADD_ENUM_TO_MAP
}
return values[wc];
}
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#enum5-dont-use-all_caps-for-enumerators
Disagree b/c enums are constants and constants should be in all caps.
Also macro use should be minimized.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#enum8-specify-enumerator-values-only-when-necessary
Agreed with caveat that the initial value should always be specified to anchor the values to a known value. Otherwise the compiler may choose any random value as the anchor value of the enum. This becomes a problem when it becomes necessary to serialize/deserialize the value reliably.
============================================================================================================================================================================================================
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r1-manage-resources-automatically-using-resource-handles-and-raii-resource-acquisition-is-initialization
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r4-a-raw-reference-a-t-is-non-owning
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r5-prefer-scoped-objects-dont-heap-allocate-unnecessarily
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r12-immediately-give-the-result-of-an-explicit-resource-allocation-to-a-manager-object
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r13-perform-at-most-one-explicit-resource-allocation-in-a-single-expression-statement
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r20-use-unique_ptr-or-shared_ptr-to-represent-ownership
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r31-if-you-have-non-std-smart-pointers-follow-the-basic-pattern-from-std
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r34-take-a-shared_ptrwidget-parameter-to-express-that-a-function-is-part-owner
Agreed
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r21-prefer-unique_ptr-over-shared_ptr-unless-you-need-to-share-ownership
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r22-use-make_shared-to-make-shared_ptrs
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r23-use-make_unique-to-make-unique_ptrs
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r24-use-stdweak_ptr-to-break-cycles-of-shared_ptrs
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r32-take-a-unique_ptrwidget-parameter-to-express-that-a-function-assumes-ownership-of-a-widget
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r36-take-a-const-shared_ptrwidget-parameter-to-express-that-it-might-retain-a-reference-count-to-the-object-
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r37-do-not-pass-a-pointer-or-reference-obtained-from-an-aliased-smart-pointer
Fair enough
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r2-in-interfaces-use-raw-pointers-to-denote-individual-objects-only
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r3-a-raw-pointer-a-t-is-non-owning
When possible don't use raw pointers but smart pointers instead. When not possible, agreed.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r6-avoid-non-const-global-variables
Agreed; though there are exceptions
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r10-avoid-malloc-and-free
Agreed with respect to classes.
Disagreed with respect to structs - structs are pure data and should be available to C code too, so use C rules with them, not C++ rules. Similarly allocate them using C allocation/deallocation methods.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r11-avoid-calling-new-and-delete-explicitly
Mixed - new/delete need to be used for resource management; but pointers should be managed via smart pointers as much as possible.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r14-avoid--parameters-prefer-span
Moot b/c it's a C++20 thing
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r15-always-overload-matched-allocationdeallocation-pairs
Agreed - though better yet, don't overload them.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r30-take-smart-pointers-as-parameters-only-to-explicitly-express-lifetime-semantics
Kinda agree - smart pointers provide a nice `is valid` checking capability so pointers should be encapsulated by them as much as possible.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r33-take-a-unique_ptrwidget-parameter-to-express-that-a-function-reseats-thewidget
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r35-take-a-shared_ptrwidget-parameter-to-express-that-a-function-might-reseat-the-shared-pointer
Not sure
============================================================================================================================================================================================================
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es3-dont-repeat-yourself-avoid-redundant-code
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es5-keep-scopes-small
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es12-do-not-reuse-names-in-nested-scopes
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es26-dont-use-a-variable-for-two-unrelated-purposes
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es31-dont-use-macros-for-constants-or-functions
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es32-use-all_caps-for-all-macro-names
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es33-if-you-must-use-macros-give-them-unique-names
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es40-avoid-complicated-expressions
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es43-avoid-expressions-with-undefined-order-of-evaluation
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es44-dont-depend-on-order-of-evaluation-of-function-arguments
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es45-avoid-magic-constants-use-symbolic-constants
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es47-use-nullptr-rather-than-0-or-null
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es56-write-stdmove-only-when-you-need-to-explicitly-move-an-object-to-another-scope
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es61-delete-arrays-using-delete-and-non-arrays-using-delete
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es77-minimize-the-use-of-break-and-continue-in-loops
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es79-use-default-to-handle-common-cases-only
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es84-dont-try-to-declare-a-local-variable-with-no-name
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es85-make-empty-statements-visible
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es86-avoid-modifying-loop-control-variables-inside-the-body-of-raw-for-loops
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es100-dont-mix-signed-and-unsigned-arithmetic
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es101-use-unsigned-types-for-bit-manipulation
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es106-dont-try-to-avoid-negative-values-by-using-unsigned
Agreed
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es24-use-a-unique_ptrt-to-hold-pointers
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es60-avoid-new-and-delete-outside-resource-management-functions
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es63-dont-slice
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es103-dont-overflow
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es104-dont-underflow
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es105-dont-divide-by-integer-zero
Fair enough
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es1-prefer-the-standard-library-to-other-libraries-and-to-handcrafted-code
Mixed....STL code is often hard to parse because of its style. It's also hard to examine under a debugger without specialized helpers - helpers you often have to write yourself b/c no debugger provides them. Now, it's more true now than it has been in the past about the "most widely known and best tested"; but it still comes down to "is it the right fit?" If it's not the right fit, don't use it.
Also, there are some concepts in the STL that are backwards (std::deque vs std::list).
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es2-prefer-suitable-abstractions-to-direct-use-of-language-features
Mixed - and their example is a good reason why. Their "good" example is not easy to understand unless you know the specific language features uses. The longer version is actually easier to understand - though I wouldn't have written it that way either:
std::deque<string> read2(istream& is)
{
// data to send back
std::deque<string> returnValue;
// single object for the loop instead of redeclaring it on each iteration
std::string s;
while (true) {
// read a value
s << is;
// if data came back, save it for return
if s.length() > 0 {
returnValue.push_back(s);
s.clear();
continue;
}
// otherwise break out
break
}
// send the data back to the caller
return returnValue;
}
Now if you actually compare what they wrote in their examples you have two very different functions - the first, like the above, just pulls strings and returns them with each pull adding another entry into the stack; while the second reads up to N characters and does the same in a more C-like manner.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es6-declare-names-in-for-statement-initializers-and-conditions-to-limit-scope
Agreed - but don't misuse a for-loop just for this purpose. Sometimes it's useful to just declare an anonymous block for this purpose:
func x() {
{
int a = 5;
...
}
{
double a = 20.0;
...
}
}
The block creates a minimized scope.
Caveat: Do NOT declare a variable in an If or Switch statement; assignments in conditionals has always been a long standing place for bugs.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es7-keep-common-and-local-names-short-and-keep-uncommon-and-non-local-names-longer
Disagree - make names useful/meaningful. If a short name is meaningful fine; but don't be afraid to use a long name to properly convey meaning.
Compilers no longer have any meaningful limitation on name sizes, so keep things meaningful.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es8-avoid-similar-looking-names
Mixed - if there's meaning fine; if it's not adding value then yes, don't do it.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es9-avoid-all_caps-names
Disagree - constants should be in all caps; enums are essentially constants so they can be in all caps too. This is done to distinguish them easily from modifiable variables.
Historically C Macros constants were in all caps; but not all C macros were in all caps. Macros are mixed.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es10-declare-one-name-only-per-declaration
Agreed - I'd even go so far as to say only one variable declaration per line:
int a, b, c; // yuck
// easier to move around when needed
int a;
int b;
int c;
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es11-use-auto-to-avoid-redundant-repetition-of-type-names
Mixed - `auto` has very limited uses. I do agree that using it for things like iterators it useful, but that's primarily also because the scope and usage also conveys that it's an iterator and not some random type. Types are important; use them.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es20-always-initialize-an-object
Agreed - in general always initialize; unless there's a specific reason not to - in which case document it with a comment explaining why.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es21-dont-introduce-a-variable-or-constant-before-you-need-to-use-it
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es22-dont-declare-a-variable-until-you-have-a-value-to-initialize-it-with
Mixed - but this is partly historical; strict C required variables to be declared en-bloc before any code. While there was a technical reason at the time; it actually also provided the value that it was easy to know where things were defined so one could easily know where to look to determine the types of things. True, many use IDEs with Intellisense now - but many also do not. I find it's still useful to do.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es23-prefer-the--initializer-syntax
Disagree - actually, avoid them. They make code harder to read.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es25-declare-an-object-const-or-constexpr-unless-you-want-to-modify-its-value-later-on
Mixed - one can go overboard with const/constexpr declarations; and it just makes for lots of refactoring when it needs to be changed. Be reasonable; declare things const/constexpr when it's known for certain it won't ever be changed.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es27-use-stdarray-or-stack_array-for-arrays-on-the-stack
Mixed - use appropriate data structures. There is never a hard fast rule. std::deque is probably a better fit than std::array.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es28-use-lambdas-for-complex-initialization-especially-of-const-variables
I guess this can make sense. but actual functions would be better since they'd be easier to unit test for correctness.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es30-dont-use-macros-for-program-text-manipulation
Mixed - in general yes avoid Macros when at all possible; however, their stringify example is actually better done with macros:
enum class webColor {
red=0xFF0000,
green=0x00FF00,
blue=0x0000FF
};
std::string webColorToString(webColor wc) {
static bool initialized = false;
static std::map<webColor, std::string> values;
if !initialized {
// define a macro to add stuff to the map since the macro language
// makes it easy to define the string value
#define ADD_ENUM_TO_MAP(v) ( //
values[v] = std::string(#v); //
)
ADD_ENUM_TO_MAP(red)
ADD_ENUM_TO_MAP(blue)
ADD_ENUM_TO_MAP(green)
// clean up the macro namespace when done
#undef ADD_ENUM_TO_MAP
}
return values[wc];
}
Why? Switches have limit on how many branches can be used, and writing many many if blocks is just error prone itself. A simple macro like above makes things very clear. Also, notice that the macro is limited in scope - it's undefined as soon as it's no longer needed.
NOTE: The above stringify actually has better performance than doing massive if-blocks and is on-par with switches but doesn't have the limitations.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#-es34-dont-define-a-c-style-variadic-function
Mixed - limit usage of variadics, but if you need to use variadics then use va_args style.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es41-if-in-doubt-about-operator-precedence-parenthesize
Better rule: Always paranthesize. Make the intent explicit.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es42-keep-use-of-pointers-simple-and-straightforward
Mixed - should one _always_ be using pointer arithmetic? No. Should one always avoid it? No. Pointer arithmetic is far faster than calling functions to do something similar - whether that's an iterator or otherwise. If pointer arithmetic is needed, then be sure to document what's going on and why.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es46-avoid-lossy-narrowing-truncating-arithmetic-conversions
If you need to do this, then document it to make sure the next reader can understand the intent.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es48-avoid-casts
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es49-if-you-must-use-a-cast-use-a-named-cast
Don't avoid, but use appropriately and document why. If possible, use the static_cast/const_cast/dynamic_cast/reinterpret_cast/etc too and be sure to document it.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es50-dont-cast-away-const
Agreed - if you can; but sometimes you can't help it, in which case document it.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es55-avoid-the-need-for-range-checking
Eh....may be.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es64-use-the-tenotation-for-construction
For the moment at least, I'm going to disagree and say we should stick with the T() method of construction.
I may be convinced otheerwise; but that can be up for discussion.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es65-dont-dereference-an-invalid-pointer
Agreed; though not necessarily on their recommended methods - best to pass around smart pointers so local code can do `isNull()` checking on the smart pointer.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es70-prefer-a-switch-statement-to-an-if-statement-when-there-is-a-choice
Agreed, though types will often force one over the other.
Another alternative is using std::map<> for type conversions (int to string) or handling callbacks based on key values.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es71-prefer-a-range-for-statement-to-a-for-statement-when-there-is-a-choice
Disagree - the `range-for` is abnormal and introduces inconsistency in reading that will require too much explanation.
This would have been better if they had followed other languages: `for x in v` as opposed to `for x : v`.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es72-prefer-a-for-statement-to-a-while-statement-when-there-is-an-obvious-loop-variable
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es73-prefer-a-while-statement-to-a-for-statement-when-there-is-no-obvious-loop-variable
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es74-prefer-to-declare-a-loop-variable-in-the-initializer-part-of-a-for-statement
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es75-avoid-do-statements
Use the appropriate loop type based on the context:
- pre-condition: while{}
- post-condition: do {} while
- index/iterator: for(;;) {}
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es76-avoid-goto
Agreed as a general rule - there are exceptions to the rule though.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es78-dont-rely-on-implicit-fallthrough-in-switch-statements
Agreed - any fall through should have a comment documenting that it was intended to fall through
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es87-dont-add-redundant--or--to-conditions
Disagree - be explicit
if (p) {} // relies on implicit conversions, doesn't documented intended vale or type of `p`
assembler????
if (p != 0) {} // explicit - we know we're looking for a non-zero integer value
cmp p, 0
jz skip_code
if (p == 0) {} // explicit - we know we're looking for a zero integer value
cmp p, 0
jnz skip_code
The compiler may optimize to the same function as an optimization - that's fine; but let's leave intact the ability to debug and know what to expect.
RULE: Explicit is better than implicit, even if it's more text.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es102-use-signed-types-for-arithmetic
In general yes; but dont' cast to signed just to do arithmetic. Follow the requirements of the value.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es107-dont-use-unsigned-for-subscripts-prefer-gslindex
Disagree - Indexes should always be unsigned; and auto shouldn't be used to refer to an index. `-1` shouldn't be used to refer to a bad index - yeah, I know tons of APIs do it.
I'd also say we should avoid GSL - it's not standard, it's not ISO, and it's not provided by the compiler. Documentation on GSL is also hard to come by and the name is confusing - GNU Scientific Library, Microsoft GSL, etc.
In addition, I don't think GSL adds sufficient value at this time.
============================================================================================================================================================================================================
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#per7-design-to-enable-optimization
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#per13-eliminate-redundant-indirections
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#per14-minimize-the-number-of-allocations-and-deallocations
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#per15-do-not-allocate-on-a-critical-branch
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#per16-use-compact-data-structures
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#per17-declare-the-most-used-member-of-a-time-critical-struct-first
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#per18-space-is-time
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#per19-access-memory-predictably
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#per30-avoid-context-switches-on-the-critical-path
Agreed
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#per1-dont-optimize-without-reason
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#per2-dont-optimize-prematurely
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#per3-dont-optimize-something-thats-not-performance-critical
Mixed. Personally I'm of the opinion that we should optimize all the time and that code shouldn't degrade in performance over time. Code that ran 30, 40 years ago should still be used today. An i386/i486/Pentium
should be able to do 90% of our applications today and not see performance impeded compared to when they were in their prime - that doesn't mean that all features would necessarily be available, but often
code is modified and made less efficient as new features are added impeding older systems from being able to perform when simpler methods of enabling/disabling functionality would have sufficed.
You generally don't know that something is _not_ performance critical until the application is analyzed - people are generally bad at identifying performance critical sections of code.
However, excessive time shouldn't be spent on optimizing everything - do a good job, make it efficient, and move on quickly.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#per4-dont-assume-that-complicated-code-is-necessarily-faster-than-simple-code
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#per5-dont-assume-that-low-level-code-is-necessarily-faster-than-high-level-code
And vice versa. Badly written code is badly written - whether it's complex or simple, high level or low level.
For our purposes aim to write maintainable, easily readable code and heavily document where things need to change up for performance.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#per6-dont-make-claims-about-performance-without-measurements
Don't make definitive assertions without measurements and tests to back them up. We can certainly talk about the expected performance impact as things go, etc.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#per10-rely-on-the-static-type-system
Agreed - when possible. There are cases where one has to break the static type system, but it should be well documented in those cases.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#per11-move-computation-from-run-time-to-compile-time
Put computation where it makes the most sense. It's not an either/or; one is not better than the other.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#per12-eliminate-redundant-aliases
Just avoid aliases. There's no need for them.
============================================================================================================================================================================================================
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp1-assume-that-your-code-will-run-as-part-of-a-multi-threaded-program
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp2-avoid-data-races
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp3-minimize-explicit-sharing-of-writable-data
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp4-think-in-terms-of-tasks-rather-than-threads
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp8-dont-try-to-use-volatile-for-synchronization
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp9-whenever-feasible-use-tools-to-validate-your-concurrent-code
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp20-use-raii-never-plain-lockunlock
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp21-use-stdlock-or-stdscoped_lock-to-acquire-multiple-mutexes
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp22-never-call-unknown-code-while-holding-a-lock-eg-a-callback
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp23-think-of-a-joining-thread-as-a-scoped-container
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp24-think-of-a-thread-as-a-global-container
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp32-to-share-ownership-between-unrelated-threads-use-shared_ptr
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp40-minimize-context-switching
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp41-minimize-thread-creation-and-destruction
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp43-minimize-time-spent-in-a-critical-section
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp44-remember-to-name-your-lock_guards-and-unique_locks
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp50-define-a-mutex-together-with-the-data-it-guards-use-synchronized_valuet-where-possible
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp100-dont-use-lock-free-programming-unless-you-absolutely-have-to
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp101-distrust-your-hardwarecompiler-combination
Agreed
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp42-dont-wait-without-a-condition
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp60-use-a-future-to-return-a-value-from-a-concurrent-task
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp61-use-async-to-spawn-concurrent-tasks
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp110-do-not-write-your-own-double-checked-locking-for-initialization
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp111-use-a-conventional-pattern-if-you-really-need-double-checked-locking
Fair enough
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp51-do-not-use-capturing-lambdas-that-are-coroutines
Moot - avoid lambdas
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp25-prefer-gsljoining_thread-over-stdthread
Disagree - we should avoid GSL right now. From another section:
I'd also say we should avoid GSL - it's not standard, it's not ISO, and it's not provided by the compiler. Documentation on GSL is also hard to come by and the name is confusing - GNU Scientific Library, Microsoft GSL, etc.
In addition, I don't think GSL adds sufficient value at this time.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp26-dont-detach-a-thread
Agreed; an object should manage the thread:
struct threadManager {
std::thread theThread;
};
The object can then be a member variable of another object, global scope, etc - wherever the thread needs to be managed by. If the owner of the object goes away then the thread should be terminated.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp31-pass-small-amounts-of-data-between-threads-by-value-rather-than-by-reference-or-pointer
Agreed, if possible. But often more is necessary than simple values.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp102-carefully-study-the-literature
sure, if you have time
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp200-use-volatile-only-to-talk-to-non-c-memory
Better yet - avoid volatile; it's basically just flag to the compiler saying the value is unstable and shouldn't be cached for very long.
============================================================================================================================================================================================================
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e1-develop-an-error-handling-strategy-early-in-a-design
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e4-design-your-error-handling-strategy-around-invariants\
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e7-state-your-preconditions
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e8-state-your-postconditions
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e13-never-throw-while-being-the-direct-owner-of-an-object
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e16-destructors-deallocation-and-swap-must-never-fail
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e27-if-you-cant-throw-exceptions-use-error-codes-systematically
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e28-avoid-error-handling-based-on-global-state-eg-errno
Agreed
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e2-throw-an-exception-to-signal-that-a-function-cant-perform-its-assigned-task
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e3-use-exceptions-for-error-handling-only
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e30-dont-use-exception-specifications
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e31-properly-order-your-catch-clauses
Normally, I'd disagree and say C++ Exceptions should be avoided; however since we're integrating with Python and Exceptions are required so agreed.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e14-use-purpose-designed-user-defined-types-as-exceptions-not-built-in-types
Use the Exception hierarchy of the built-in exceptions and extend for custom types. Where it makes sense use the built-in types; but more often than not a custom exception will provide better
detail - but keep the custom exceptions to a minimum.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e5-let-a-constructor-establish-an-invariant-and-throw-if-it-cannot
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e15-catch-exceptions-from-a-hierarchy-by-reference
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e19-use-a-final_action-object-to-express-cleanup-if-no-suitable-resource-handle-is-available
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e25-if-you-cant-throw-exceptions-simulate-raii-for-resource-management
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e26-if-you-cant-throw-exceptions-consider-failing-fast
Fair Enough
Disagree - Constructors should never throw. Constructors should be able to default to an error state that will cause anything else to throw because the object isn't in the right state.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e6-use-raii-to-prevent-leaks
Agree in principle, but Mixed on implementation. Resources must not be leaked.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e12-use-noexcept-when-exiting-a-function-because-of-a-throw-is-impossible-or-unacceptable
Fair enough; though adding `noexcept` can itself cause issues that may lead to some unnecessary churn in the code.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e17-dont-try-to-catch-every-exception-in-every-function
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e18-minimize-the-use-of-explicit-trycatch
Disagree - find the right balance. The closer an exception is caught to where it was thrown the more likely it will be able to be handled and code will be able to recover instead of having to crash the application since the it has no ability to recover.
============================================================================================================================================================================================================
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#con1-by-default-make-objects-immutable
Agreed.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#con3-by-default-pass-pointers-and-references-to-consts
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#con5-use-constexpr-for-values-that-can-be-computed-at-compile-time
Fair enough
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#con2-by-default-make-member-functions-const
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#con4-use-const-to-define-objects-with-values-that-do-not-change-after-construction
Agreed in principle; however, this often causes unnecessary code churn
============================================================================================================================================================================================================
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t62-place-non-dependent-class-template-members-in-a-non-templated-base-class
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t81-do-not-mix-hierarchies-and-arrays
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t120-use-template-metaprogramming-only-when-you-really-need-to
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t141-use-an-unnamed-lambda-if-you-need-a-simple-function-object-in-one-place-only
Agreed
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t69-inside-a-template-dont-make-an-unqualified-non-member-function-call-unless-you-intend-it-to-be-a-customization-point
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t83-do-not-declare-a-member-function-template-virtual
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t84-use-a-non-template-core-implementation-to-provide-an-abi-stable-interface
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t123-use-constexpr-functions-to-compute-values-at-compile-time
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t124-prefer-to-use-standard-library-tmp-facilities
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t125-if-you-need-to-go-beyond-the-standard-library-tmp-facilities-use-an-existing-library
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t140-name-all-operations-with-potential-for-reuse
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t144-dont-specialize-function-templates
Fair enough
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t46-require-template-arguments-to-be-at-least-regular-or-semiregular
Fair enough - but really the default constructor should have been defined
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t12-prefer-concept-names-over-auto-for-local-variables
Agreed - auto should be avoided for most cases
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t1-use-templates-to-raise-the-level-of-abstraction-of-code
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t2-use-templates-to-express-algorithms-that-apply-to-many-argument-types
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t3-use-templates-to-express-containers-and-ranges
Agreed, if you must use a template. If possible, avoid templates.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t5-combine-generic-and-oo-techniques-to-amplify-their-strengths-not-their-costs
Hard to say...
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t10-specify-concepts-for-all-template-arguments
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t11-whenever-possible-use-standard-concepts
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t13-prefer-the-shorthand-notation-for-simple-single-type-argument-concepts
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t20-avoid-concepts-without-meaningful-semantics
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t21-require-a-complete-set-of-operations-for-a-concept
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t22-specify-axioms-for-concepts
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t23-differentiate-a-refined-concept-from-its-more-general-case-by-adding-new-use-patterns
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t24-use-tag-classes-or-traits-to-differentiate-concepts-that-differ-only-in-semantics
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t25-avoid-complementary-constraints
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t26-prefer-to-define-concepts-in-terms-of-use-patterns-rather-than-simple-syntax
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t41-require-only-essential-properties-in-a-templates-concepts
Moot due to compiler support for our platforms
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t40-use-function-objects-to-pass-operations-to-algorithms
Mixed - yes, passing objects with functions attached is better, and passing data objects is better than passing tons of parameters (easier to read).
However, passing function objects leads to the bad aspects of encouraging lambda functions, which yields bad code that's harder to read, understand, and even harder to test.
Furthermore, it would be very hard to make a pointer be worse than passing an object so their performance claim is just outright bad. At a minimum a lambda function or function object *is* a function pointer itself - you can't get around that.
Ultimately since this encourages usage of lambda's I'm going to have to say it should be avoided in favor of class objects or structures.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t42-use-template-aliases-to-simplify-notation-and-hide-implementation-details
Disagree - verbosity isn't bad; however, there's a safer way to to solve this - just make the iterator a parameter of the template:
template<typename T, typename I = t::iterator>
void user(T& c) {
...
}
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t43-prefer-using-over-typedef-for-defining-aliases
Disagree - `using` should only be used in implementation files and only once per implementation file at that.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t44-use-function-templates-to-deduce-class-template-argument-types-where-feasible
Disagree - explicit is better than implicit. We used typed languages to favor being explicit about types.
Certainly capture all the types in the template declaration; but do not leave it to the compiler to figure it out.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t47-avoid-highly-visible-unconstrained-templates-with-common-names
Agreed - though generally I prefer to be explicit even with templates where possible.
With a function template that's not too possible; but with a class it is by typedefing the class out before using it - again, explicit favors implicit.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t48-if-your-compiler-does-not-support-concepts-fake-them-with-enable_if
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t121-use-template-metaprogramming-primarily-to-emulate-concepts
Disagree - if the compiler baseline doesn't support them then don't use them. Don't play games with the compiler. That causes more issues than it's ever worth.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t49-where-possible-avoid-type-erasure
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t67-use-specialization-to-provide-alternative-implementations-for-irregular-types
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t82-linearize-a-hierarchy-when-virtual-functions-are-undesirable
Uhh...not enough information here to really say one way or another.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t60-minimize-a-templates-context-dependencies
Mixed - this can often be solved by proper unit testing and making sure that any type that can go through the template is unit tested.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t61-do-not-over-parameterize-members-scary
Agreed, and no - that's not scary it's proper object design. Embedded Types are more scary and result in worse code.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t64-use-specialization-to-provide-alternative-implementations-of-class-templates
Agreed - this is best exemplified with class templates:
template<typename T>
class X {
}
class Y : X<int> {
// functionality specific to the X<int> version that shouldn't be part of the generic X
}
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t65-use-tag-dispatch-to-provide-alternative-implementations-of-a-function
Not sure. Probably better to find a solution without templates at all.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t68-use--rather-than--within-templates-to-avoid-ambiguities
May be.. but generally avoid `{}` for construction.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t80-do-not-naively-templatize-a-class-hierarchy
Mixed - defined what you need to in the template. The cost is compilation time; the optimizer will remove unused code so it won't be a run-time issue.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t100-use-variadic-templates-when-you-need-a-function-that-takes-a-variable-number-of-arguments-of-a-variety-of-types
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t101--how-to-pass-arguments-to-a-variadic-template-
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t102-how-to-process-arguments-to-a-variadic-template
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t103-dont-use-variadic-templates-for-homogeneous-argument-lists
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t142-use-template-variables-to-simplify-notation
Honestly, va_args isn't that hard a thing to deal with. Don't be afraid of it; it should also be the exception to the rule.
Variadic Templates don't make things any easier, so stick with va_args in the few cases it's needed, namely in a printf style function.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t122-use-templates-usually-template-aliases-to-compute-types-at-compile-time
Not sure - there really isn't enough information to know what they mean here.
If they're meaning:
typedef X<t> Y;
Y z;
Then yes agreed. If they mean something else...well don't know.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t141-use-an-unnamed-lambda-if-you-need-a-simple-function-object-in-one-place-only
Mixed - in general lambdas should be avoided. However, there is an occasional use where the lambda is super simplified:
Qt::Connect(this->buttonOk, SIGNAL(clicked()), this, SLOT([](){ this->doClick(); }));
// or something like that...note the use of the lambda is only to pull in another related method that can't be used as a slot
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t143-dont-write-unintentionally-non-generic-code
Mixed; at a minimum they have really bad examples.
- don't use auto for non-iterator-based loops
- the `!=` comparison can actually lead to bad loops, which is why `<` is used, especially for counting-based loops otherwise an improper increment of the indexed variable could lead to the loop running through the entire capacity of the type. However, if it's iterator-based looping then != is needed to test for reaching the end of the iteration.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#t150-check-that-a-class-matches-a-concept-using-static_assert
If this is compile time only, okay; otherwise...avoid asserts.
============================================================================================================================================================================================================
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cpl1-prefer-c-to-c
well, of course that's what the C++ folks will say. However, C actually does better in many cases since it doesn't have virtual tables, and functions are not implicitly called (e.g constructors). This is where defining the interfaces appropriately comes in - if you need to define a C API then certainly make the functions C-Style and declare them as such:
extern "C" {
// C API here
}
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cpl2-if-you-must-use-c-use-the-common-subset-of-c-and-c-and-compile-the-c-code-as-c
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cpl3-if-you-must-use-c-for-interfaces-use-c-in-the-calling-code-using-such-interfaces
Agreed
============================================================================================================================================================================================================
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf2-a-h-file-must-not-contain-object-definitions-or-non-inline-function-definitions
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf3-use-h-files-for-all-declarations-used-in-multiple-source-files
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf4-include-h-files-before-other-declarations-in-a-file
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf5-a-cpp-file-must-include-the-h-files-that-defines-its-interface
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf7-dont-write-using-namespace-at-global-scope-in-a-header-file
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf8-use-include-guards-for-all-h-files
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf9-avoid-cyclic-dependencies-among-source-files
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf10-avoid-dependencies-on-implicitly-included-names
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf11-header-files-should-be-self-contained
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf20-use-namespaces-to-express-logical-structure
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf21-dont-use-an-unnamed-anonymous-namespace-in-a-header
Agreed
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf1-use-a-cpp-suffix-for-code-files-and-h-for-interface-files-if-your-project-doesnt-already-follow-another-convention
Agreed - keep it simple:
.h -> header files
.c -> C implementation files
.cpp -> C++ implementation files
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf6-use-using-namespace-directives-for-transition-for-foundation-libraries-such-as-std-or-within-a-local-scope-only
Mixed
- `using namespace` should only be used for an implementation file's specific namespace (e.g local scope)
- any external entity should just have it's full namespace specified.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf12-prefer-the-quoted-form-of-include-for-files-relative-to-the-including-file-and-the-angle-bracket-form-everywhere-else
Agreed; though I'd say only files that are explicitly next to the implementation should use the quoted form.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf22-use-an-unnamed-anonymous-namespace-for-all-internalnon-exported-entities
Mixed - yes, using a namespace is a good thing; however, don't make it anonymouse. Use `internal` or another term to denote it's file local.
============================================================================================================================================================================================================
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sl1--use-libraries-wherever-possible
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sl2-prefer-the-standard-library-to-other-libraries
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sl3-do-not-add-non-standard-entities-to-namespace-std
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sl4-use-the-standard-library-in-a-type-safe-manner
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon3-avoid-bounds-errors
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slstr1-use-stdstring-to-own-character-sequences
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slstr3-use-zstring-or-czstring-to-refer-to-a-c-style-zero-terminated-sequence-of-characters
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slstr10-use-stdstring-when-you-need-to-perform-locale-sensitive-string-operations
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slio2-when-reading-always-consider-ill-formed-input
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slregex-regex
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slchrono-time
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slc1-dont-use-setjmplongjmp
Agreed - with the exception that use of Boost is equally acceptable.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slstr2-use-stdstring_view-or-gslspanchar-to-refer-to-character-sequences
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slstr5-use-stdbyte-to-refer-to-byte-values-that-do-not-necessarily-represent-characters
Agreed - though moot b/c we're C++11
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slstr4-use-char-to-refer-to-a-single-character
Fair enough
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon1-prefer-using-stl-array-or-vector-instead-of-a-c-array
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon2-prefer-using-stl-vector-by-default-unless-you-have-a-reason-to-use-a-different-container
I get where they're coming from; but honestly use the best thing for what is being done.
BTW, std::deque is often better to use than std::vector.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon4-dont-use-memset-or-memcpy-for-arguments-that-are-not-trivially-copyable
Agreed though I'd go one further - only use memcpy/memset on C structures/types; avoid them on C++ objects.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slstr11-use-gslspanchar-rather-than-stdstring_view-when-you-need-to-mutate-a-string
Disagree - stick to STL. Let's not use the GSL. Do use std::string_view for only read-only strings, and std::string for read-write strings.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slio1-use-character-level-input-only-when-you-have-to
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slio3-prefer-iostreams-for-io
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slio10-unless-you-use-printf-family-functions-call-ios_basesync_with_stdiofalse
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slio50-avoid-endl
C++'s IOStream functionality is overly complex. It's actually easier to just use C's I/O functionality.
Further, iostream is complex enough that it would be a higher security risk due to complexity versus the simplicity of the printf family.
============================================================================================================================================================================================================
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#a2-express-potentially-reusable-parts-as-a-library
Agreed
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#a1-separate-stable-code-from-less-stable-code
Fair enough
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#a4-there-should-be-no-cycles-among-libraries
Agreed - if a cyclic is needed, define an interface (abstract class); however, that should be the exception to the rule.
============================================================================================================================================================================================================
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nr6-dont-place-all-cleanup-actions-at-the-end-of-a-function-and-goto-exit
Agreed
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nr1-dont-insist-that-all-declarations-should-be-at-the-top-of-a-function
Fair enough
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nr2-dont-insist-to-have-only-a-single-return-statement-in-a-function
Disagree - single-entry/single-exit actually helps ensure good logic in many cases. Often it can show algorithms that are simply better and
more expandable.
For instance, in their example, don't use the if-block structure - that's limited in size. Instead use an std::map with the value as a key.
IOW, multi-exit almost always points to needing to re-evaluate the function as there's a better, more efficient means of achieving the same thing
that is actually cleaner and easier to understand. There are exceptions.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nr3-dont-avoid-exceptions
I'd disagree except we have to use Exceptions with Python.
Biggest issue is lazy programming that doesn't handle the errors close enough to actually fix the issue.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nr4-dont-insist-on-placing-each-class-declaration-in-its-own-source-file
Disagree. We don't care how many files we have. Having only one class in a header/implementation file helps with locality and keeping information shorter so it's easier to understand.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nr5-dont-use-two-phase-initialization
If you can avoid it, yet; but you don't always get a choice.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nr7-dont-make-all-data-members-protected
Mixed - yes, not everything should be protected. But not everything should be private either, and absolutely nothing should be public in a class.
Remember, protected is for access by derived classes only. Use it for values/types that need to be exposed to the derived objects.
Sometimes this is also useful for creating mocks and validating the functionality works as expected.
============================================================================================================================================================================================================
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#gsl-guidelines-support-library
Disgree - let's avoid the library.
============================================================================================================================================================================================================
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nl2-state-intent-in-comments
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nl3-keep-comments-crisp
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nl5-avoid-encoding-type-information-in-names
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nl8-use-a-consistent-naming-style
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nl15-use-spaces-sparingly
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nl18-use-c-style-declarator-layout
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nl20-dont-place-two-statements-on-the-same-line
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nl21-declare-one-name-only-per-declaration
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nl26-use-conventional-const-notation
Agreed
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nl7-make-the-length-of-a-name-roughly-proportional-to-the-length-of-its-scope
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nl11-make-literals-readable
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nl17-use-kr-derived-layout
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nl25-dont-use-void-as-an-argument-type
Fair enough
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nl9-use-all_caps-for-macro-names-only
Macros, enums, and constants should be the only things to have all caps.
Enums should be scoped to a namespace as well.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nl1-dont-say-in-comments-what-can-be-clearly-stated-in-code
Disagree - the value of comments is that they carry the intent of what is to be implemented, not simply the implementation.
When the next dev comes along they can tell what the implementation is by reading the code but they can't tell what the previous dev intended to do,
and the two may not be the same. However, this doesn't mean that every line of code needs to be documented. Break it into logical blocks, even within a C/C++ block.
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nl4-maintain-a-consistent-indentation-style
Agreed - though one further, always use braces
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nl16-use-a-conventional-class-member-declaration-order
Agreed - public/protected/private ordering for better ABI compatibility
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nl19-avoid-names-that-are-easily-misread
Agreed - there's an old story of a Windows developer that named a variable `hItem` (a handle for an item) and his wife read it as "hit 'em" and told
him not to be so violent in his code.
@ministerofinformation
Copy link

Gave this a quick once over - the only thing that immediately popped out at me was that I'd characterize the response to

http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp200-use-volatile-only-to-talk-to-non-c-memory

a little differently - the volatile keyword affects more than just allowing/disallowing register allocation in terms of how it impacts the soundness and applicability of other compiler transformations and memory-space visibility - but I agree with the general sentiment that we shouldn't be encountering too many cases where the volatile keyword is what should be used; in many of the cases where the value is subject to external modification, there should presumably be other mechanisms/interfaces at play (e.g. atomic<>) to ensure that the semantics of the access are sensible, rather than just "visible", although there will be some cases that are directly exposed as raw shared memory or proxies for HW values, etc.

As an aside, I don't think I'm as anti-lambda, but I blame my time working with Scheme and Common Lisp and don't see too much reason for us to be using them in our C++ code ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment