Created
July 15, 2021 03:43
-
-
Save BenjamenMeyer/a3454e4dce1787af30e60f7180b650d1 to your computer and use it in GitHub Desktop.
Review of ISOCPP Guidelines
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 ;-)