A lot of copy paste is still inside some of my projects, too. Well yes, I understand. Starting to understand C++ and then be confident requires years. And good code examples, and advice on that path are often priceless. But you also know that without proper C++ education and sound grasp of key concepts, copy paste will not take you very far. You are capable of abstract thinking and you want to deliver solid C++ code.
So let me show you how applying this kind of thinking can produce much better C++ while refactoring your daily copy/paste dose from Stack Overflow.
So here we go.
As a random starting point, we shall take this article from good old Wikipedia. You have decided to design your C++ code before coding and that is commendable. You have also decided, based perhaps on some Stack Overflow comment by someone, to investigate this “Policy Based Design”. The article is pretty clean and simple and it also contains an obligatory C++ code example. Ready to be slurped up and incorporated into your design.
For the purpose of this post, I will focus only on the HelloWorld class as presented in there, and take you trough thinking AND refactoring steps to considerably improve the result. In both clarity and usability of the current last release in here.
Ok, this is the original code.
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 |
template <typename OutputPolicy, typename LanguagePolicy> class HelloWorld : private OutputPolicy, private LanguagePolicy { using OutputPolicy::print; using LanguagePolicy::message; public: // Behavior method void run() const { // Two policies two methods print(message()); } }; |
The article explains the rest. We focus on the code above. What I have noticed and disliked first is the API this produces.
It requires users to compose the class from other classes, by inheritance. Before being able to use anything. And as we all know class inheritance is a bad thing.
And this is even less logical when we apply our thinking and we realize that default language is English and that HelloWorld is very often just used as it is, without switching to other languages. So let us “design in”, this realization we have just made.
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 |
#pragma once /* Copyright 2017 by dbj@dbj.org Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http ://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. __interface msvc keyword explained here: https://docs.microsoft.com/en-us/cpp/cpp/interface */ #ifndef _MSC_VER #error This code requires Visual C++ #endif // !_MSC_VER #if _MSC_VER < 1911 #error This code requires Visual C++ 14.1 or better #endif #define implements public #include <dbjio.h> using dbj::io::print; #include <memory> #include <string> using namespace std; namespace dbj { namespace philology { /* interfaces to writing and language policies */ __interface IWriterPolicy { void write(string); }; __interface ILanguagePolicy { string translate(string); }; // classes to be used // as default policies struct WideWriterPolicy : implements IWriterPolicy { void write(string msg) { print("%", msg); } }; struct LanguagePolicyEnglish : implements ILanguagePolicy { string translate(string msg) { return msg; } }; // german is also here as some folks will // want to make it a default language struct LanguagePolicyGerman : implements ILanguagePolicy { string translate(string msg) { return msg; } }; template < /* make English to be compiled as the default language */ typename LanguagePolicy = LanguagePolicyEnglish, /* compile wide string writer as the defualt output policy */ typename OutputPolicy = WideWriterPolicy > class HelloWorld : private OutputPolicy, private LanguagePolicy { using OutputPolicy::write; using LanguagePolicy::translate; public: // Behaviour method void run(string message) { // Two policy methods write(translate(message)); } }; inline void test() { HelloWorld<> hello{}; hello.run("Default policies"); } } // namespace philology } //namespace dbj |
While “at it”we have also realized that UNICODE is what anybody will want most of the time so we can safely introduce WideWriterPolicy as our default one. So here we are. the first step on our “think and refactor trip” has produced better API and thus improved the comfort of its users. Making them less error prone. Just instantiate the default class which in English language strings output as Unicode … No need to remember anything else, Just use and go.
A deeper dive
That was a bit of a thinking applied and a bit of a basic C++ templates knowledge used. But we can do much better than that. Going further, I have to say I do not like multiple inheritances at all. Or any kind of inheritance. “Diamond problem” and the such. Just say No. So why do we have it here?
I might be so bold to advise you to do a short detour to my post on why the class inheritance is bad. These days modern C++ templates are giving brittle composability of the solution and that is a good thing. We compose the solution out of parts and make it obvious to the user, in the code. Inheritance is not required for that. Inheritance makes the inheritors bloated with inherited stuff. It gives them a lot of what they do not need.
In this case, a classHelloWorld will inherit whatever is in the base policy classes. And nobody has guaranteed they will not grow to be large. And it uses just one method from each of them. And the policy classes inherited might grow and get heavy and complex. Making the life difficult for the developers using the inherited classHelloWorld.
And then this using declarations. Yet another C++ artifact from the past. These are so-called “access declaration”. Deprecated in modern C++, so the author has added the “using” keyword. , in essence, fighting with name resolution interlaced with template inheritance.
I think you are by now enough of the theory. Here is the next improvement.
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 |
/* No inhertiance! No name lookup issues. */ template < typename LanguagePolicy = LanguagePolicyEnglish, typename OutputPolicy = WideWriterPolicy > class HelloWorld2 { public: // Behaviour method void run( string msg) { // not made before this point; if ever. static LanguagePolicy language{}; static OutputPolicy printer{}; // Two policy methods printer.write( language.translate(msg) ); } }; |
Inheritance is removed. And suddenly it is wondrously obvious how unnecessary it was. Template arguments are just nice compile time typedefs. Since there is no inheritance we do not need qualified member access declarations to enforce the access to the required methods from the base classes. We do not need the using’s from above that is.
It is perhaps not your favorite bed-time reading but do some casual stroll through standard C++ library code. It is 99% this kind of design. Templates, default template arguments, and no inheritance. Alex Stepanov was really ahead of it’s time. And still is.
But wait! What are these two static’s inside the run() method?!
Always limit the class members
Eh? What is this C++ “salto-mortale” now, in there? Ok, let us go step by step through this last improvement. What are the “members”? Without ISO terminology and language escapades, I would propose a simple definition:
Member is anything that is not a class or object, method.
And what this has to do with that two statics above inside the method run? What other ways of using methods of Language Policy and Output Policy we have?
Well, perhaps the “default” C++ idiom, that springs to mind, might be:
|
1 2 3 4 5 6 7 8 |
LanguagePolicy lang_{} ; OutputPolicy output_{} ; public: void run( string msg ) { // Two policy methods output_.print(lang_.translate(msg)); } |
Two “normal” class members. Nicely made private and then simply used from inside run(). Here are the problems with this design.
HelloWorldclass size has grown- It got two more members.
- Each instance of that class will be that much bigger, increasing the size of the applications using it.
- Proper class members treatment in modern C++
- Moving semantics have to have code taking care of this two members also
- Are they both moveable? Swappable?
- Class copying has to contain code, taking care of this two members too
- Are they both copyable?
- Moving semantics have to have code taking care of this two members also
Ok, says you, sod this, I can make them both static. And then you think as I advised you to. Each instance of the HelloWorld class does not need her own policy instances, after all. The statics approach creates two instances per class, not per object. Ok, says I, that is better, but still, it will make the final app bigger. Many (hundreds) different objects can use these two policy objects. And not every one of them will be used at runtime. And yes, these two policy classes will also have to be resilient in the presence of multiple threads.
You are thinking and refactoring and you propose “a clever solution”:
|
1 2 3 4 5 6 7 8 |
void run( LanguagePolicy & language = LanguagePolicy{}, OutputPolicy & printer = OutputPolicy{}; ) { // Two policy methods printer.print(language.message()); } |
By using the default arguments feature you instantiate both policy class upon caller calling the run() method. Voila : No class members! My comment: this makes instances of two classes once per every call of the run() method. This can slow down the code considerably, it all depends on the time and resources required to instantiate the policy classes. No members true, but different problems introduced.
Instead, I have designed my code to do nothing until the run() method is called.
|
1 2 3 4 5 6 7 8 |
void run( ) const { // not made before this point; if ever. static LanguagePolicy language{}; static OutputPolicy printer{}; // Two policy methods printer.print(language.message()); } |
There are no new class members, and class copying and moving and constructing and destructing, all are the same as before. At runtime upon the first call, inside a run method, two static instances will be made. On each subsequent call, they will be just used. This is a good thing and this is called “lazy instantiation”. Do not make something if it is not used. Make it on first use only.
Conclusion
The code above is by no means a “final” solution. I have kept it deliberately at the safe distance from (too much) of C++14, C++17 and standard library abstractions. Yet we have learned a lot, and we have some useful code.
Say No to copy paste!
Of course, do not forget to add error checking before you publish it :)