Basic C++ & Singleton_registry - Suggestions to GrantRostig from an expert.

[grantrostig]

https://github.com/cppmsg/cpp_by_example/blob/master/designp_singleton/singleton_gof_registry.hpp
I've taken a look at the code in that header file.
As for some suggestions:

1. That .hpp file has both declarations in it as well as function definitions. I'd suggest moving those function definitions into one or more source files or prefixing them with the "inline" keyword.
2. The formatting is such that some lines have multiple statements on the same line. It's more common to see only a single statement per line and will be easier to read for people used to that - like myself.
3. I see that file has a "using namespace..." statement in it. Those pollute the namespace for any users of that header file. I suggest removing it.
4. The code uses a mix of qualified uses of standard library names and unqualified uses. Use only namespace qualified names for types from the standard library. So prefix mentions of types like "string" with "std" so those instead appear like "std::string".
5. Come up with a more consistent formatting style and use it everywhere within that file. Qt Creator can be configured to auto format to some standard styles and that could be used to help for that.
6. Remove the include of <bits/stdc++.h> from that file.
7. Specifically include the standard library header files that are needed for that header file to stand alone. So for instance, there'd be includes like for "", "", "", "".
8. Don't use "endl". That appends a newline AND forces a flush of the stream buffer. If you really want both, use the newline character ('\n') or string ("\n") and follow it with an explicit "std::flush".
9. Pick a max column width no more than like 120 for the code and then enforce that with newlines.
10. "Singleton_gof_registry::_registry" does not need to be a pointer to a "Registry". That's basically a pointer to a pointer to a dynamically allocated array. Make it just of type "Registry" and then it doesn't need to be "new" allocated on first use.
11. Don't mix uses of like "end()" member function values with "cend()" member function values.
12. If you want to provide exclusive access to one section of code so another thread can't access those same variables, those sections need to all use the same "std::mutex" (or a different mechanism than "std::mutex" has to be used instead). So add "mtx" as a private member variable of "Singleton_gof_registry" and then the three member functions which use a "std::mutex" would all use the "mtx" member variable instead.

[grantrostig]

RE: 2) && 5) Multiple Statements on one line and general formatting:
We do that to:

• fit more on the screen (being the function size recommendation in TCPL book)
• and or to deemphasize :
• obvious
• or mundane
• or debugging functionality.

Code layout format to help make columns for the eyes of the author.

On production code some such may be removed or reformatted.
We suggest any person reading the code simply download it and reformat it to their choice with a powerful IDE. Of course we accept pull requests in any format. :)

[grantrostig]

Thanks for the review, here are the other points, not addressed above.

1. This is a compact example, but otherwise yes.
   3 & 4) Yes, that was sloppy. Corrected.
   6 & 7) Yes, if I hope to have others do a code review, I should make it easy for non-"gcc on Linux" reviewers.
2. I like std::endl because I do want a flush during development and debugging. I'm aware of \n and use it when I want performance.
3. Yes, 120 chars per line does seem like all we can easily print on a piece of paper in landscape. On the other hand, my monitor allows for many more, but I do want to try land limit it to 120. The problem is where to put comments, Prior, after, or appended to the relevant line.
4. In this version, I'm using a pointer for registry because, I'm doing what the GOF proposes in the book. Yes, other approaches have advantages and drawbacks. It would be good to discuss those verbally during a meeting of CppMSG or other time.
5. I had trouble with getting everything const, but I have made that change now. Thanks for reminding me to work on it.
6. I believe my use of mutex has been fixed in the new commit. Thanks for pointing out my error.