{"id":5380,"date":"2018-01-21T13:00:59","date_gmt":"2018-01-21T13:00:59","guid":{"rendered":"http:\/\/writeasync.net\/?p=5380"},"modified":"2018-01-14T03:49:03","modified_gmt":"2018-01-14T03:49:03","slug":"refactoring-c-extract-method-using-friend-struct-this","status":"publish","type":"post","link":"http:\/\/writeasync.net\/?p=5380","title":{"rendered":"Refactoring C++: Extract Method using `friend struct This`"},"content":{"rendered":"

My day to day programming environment is C#<\/a> + Visual Studio<\/a> + ReSharper<\/a>. I like to think I’m fairly productive in this setup and after years of experience have picked up a few fast and easy ways to refactor my way into design improvements (even in legacy code<\/a>). This is unfortunately not the case with C++ where, due to a combination of my relative inexperience, the lack of tooling, and fundamental design decisions, it is just harder to do what I want.<\/p>\n

Consider Extract Method<\/a>, one of the first refactorings that I reach for when evolving a codebase. In languages like C# or Java, it is no big deal to move some code from an existing method on a class into a new private<\/code> method. Existing clients and callers of the code, being limited to the public surface area, will never know or care. C++ classes, however, are a different beast than their counterparts in newer managed languages. A C++ class definition must accurately reflect all<\/em> data members and functions — public<\/code> and private<\/code> — in order for calling code to use the class successfully. Adding a new private<\/code> function to a C++ class is therefore a significant event, requiring an update of the definition and, in turn, recompilation of all calling code.<\/p>\n

The severe implications of this led to some popular dependency management techniques known as “compilation firewalls<\/a>” AKA the “private implementation” or “pimpl idiom<\/a>.” Still, there are notable drawbacks (overhead in the form of extra code, heap allocations, and pointer indirection), and thus in practice a large amount of existing code is not pimpl-ized. This leaves us in the typical situation where we have a big ball of mud<\/a> that we want to organize into smaller, more comprehensible pieces, and yet we are thwarted as each small change requires an update to header files and class definitions that we would rather not make.<\/p>\n

While mulling over this problem recently, I came up with one solution that minimizes some of the drawbacks above while enabling a simple and mechanical Extract Method procedure — I’ll call it friend struct This<\/code> for reasons which will soon become obvious. To demonstrate it, I will use a small sample from Day 5 of Advent of Code 2017<\/a>. We’ll start with the function that returns the answer (invoked from main<\/code> somewhere):<\/p>\n

\r\n#include "JumpTable.h"\r\n\r\nint Day5A(const string& input)\r\n{\r\n    JumpTable jumpTable(input);\r\n    return jumpTable.Eval();\r\n}\r\n<\/pre>\n
Now the header file JumpTable.h<\/code> containing the required definitions:<\/p>\n
\r\n#pragma once\r\n\r\n#include <string>\r\n#include <vector>\r\n\r\nclass JumpTable\r\n{\r\nprivate:\r\n    JumpTable(const JumpTable&) = delete;\r\n    JumpTable& operator=(const JumpTable&) = delete;\r\n    std::vector<int> offsets_;\r\npublic:\r\n    JumpTable(const std::string& input);\r\n    int Eval();\r\n};\r\n<\/pre>\n
Finally, the actual code from JumpTable.cpp<\/code>:<\/p>\n
\r\n#include "JumpTable.h"\r\n#include <sstream>\r\n\r\nusing namespace std;\r\n\r\nJumpTable::JumpTable(const string& input)\r\n    : offsets_()\r\n{\r\n    vector<char> next;\r\n    for (auto it = input.cbegin(); it != input.cend(); ++it)\r\n    {\r\n        char c(*it);\r\n        switch (c)\r\n        {\r\n        case '\\r':\r\n        case '\\n':\r\n            if (!next.empty())\r\n            {\r\n                string str(&next[0], next.size());\r\n                int i = stoi(str);\r\n                offsets_.push_back(i);\r\n                next.clear();\r\n            }\r\n\r\n            break;\r\n\r\n        default:\r\n            next.push_back(c);\r\n            break;\r\n        }\r\n    }\r\n\r\n    if (!next.empty())\r\n    {\r\n        string str(&next[0], next.size());\r\n        int i = stoi(str);\r\n        offsets_.push_back(i);\r\n        next.clear();\r\n    }\r\n}\r\n\r\nint JumpTable::Eval()\r\n{\r\n    int steps = 0;\r\n    int i = 0;\r\n    while ((i >= 0) && (i < offsets_.size()))\r\n    {\r\n        int offset = offsets_[i];\r\n        ++offsets_[i];\r\n        i += offset;\r\n        ++steps;\r\n    }\r\n\r\n    return steps;\r\n}\r\n<\/pre>\n
This wouldn’t be a bad solution, if we could just get rid of that duplication in the line splitting code in the constructor. How do we do this without adding more function definitions to our header file and<\/em> without going “full pimpl”? Let’s start by adding one simple line to the JumpTable<\/code> class definition:<\/p>\n
\r\n\/\/...\r\nclass JumpTable\r\n{\r\nprivate:\r\n    friend struct This;\r\n\/\/...\r\n<\/pre>\n
The idea is that we’re going to implement a completely private struct<\/code> — defined above as an incomplete type<\/a> — which is a friend<\/code><\/a>, and thus given full access to the internals of the main class. We are now free to add code to our This<\/code> struct at will without ever changing the header file again! We’ll start by defining This<\/code> in the implementation file JumpTable.cpp<\/code>:<\/p>\n
\r\n\/\/...\r\nstruct This\r\n{\r\n};\r\n\/\/... JumpTable code follows\r\n<\/pre>\n
Okay, that was easy. Now let’s use Extract Method on the duplicated code in the constructor and put it in inside a new function in This<\/code>:<\/p>\n
\r\n\/\/...\r\nstruct This\r\n{\r\n    static void Process(JumpTable* This, vector<char>& next)\r\n    {\r\n        if (!next.empty())\r\n        {\r\n            string str(&next[0], next.size());\r\n            int i = stoi(str);\r\n            This->offsets_.push_back(i);\r\n            next.clear();\r\n        }\r\n    }\r\n};\r\n\r\nJumpTable::JumpTable(const string& input)\r\n    : offsets_()\r\n{\r\n    vector<char> next;\r\n\r\n    for (auto it = input.cbegin(); it != input.cend(); ++it)\r\n    {\r\n        char c(*it);\r\n        switch (c)\r\n        {\r\n        case '\\r':\r\n        case '\\n':\r\n            This::Process(this, next);\r\n            break;\r\n\r\n        default:\r\n            next.push_back(c);\r\n            break;\r\n        }\r\n    }\r\n\r\n    This::Process(this, next);\r\n}\r\n\/\/...\r\n<\/pre>\n
A few things to note:<\/p>\n
    \n
  1. This<\/code> (the struct) is just a holder for “extended” private functions of the main class. As such it has no state of its own and should not be instantiated, and will therefore declare only static methods.<\/li>\n
  2. In order for the struct to access the private state, you must pass to it the actual this<\/code> pointer of the main class.<\/li>\n
  3. Since this<\/code> is already reserved, we have to use a different name — hence This<\/code> as the pointer argument.<\/ol>\n
    The result is some slightly unusual syntax but it gets the job done; you can think of it as exposing some of the internals of what a class method actually is with the normally hidden pointer on display.<\/p>\n
    Just to make sure that this technique doesn’t have any unforeseen overhead, I experimented with Matt Godbolt<\/a>‘s excellent Compiler Explorer<\/a> to inspect the assembly listings from the code above. I compared it to the “normal” code one would write with Process<\/code> as a direct private method as follows:<\/p>\n
    \r\nclass JumpTable\r\n{\r\nprivate:\r\n    JumpTable(const JumpTable&) = delete;\r\n    JumpTable& operator=(const JumpTable&) = delete;\r\n    std::vector<int> offsets_;\r\n    void Process(std::vector<char>& next);\r\n\r\npublic:\r\n    JumpTable(const std::string& input);\r\n    int Eval();\r\n};\r\n<\/pre>\n
    I somewhat arbitrarily chose x86-64 gcc 7.2<\/strong> as the compiler and was pleased to note that the assembly output was basically identical<\/strong>. This should not be surprising, given that a private instance method under the hood is no different than a static method with an explicit this<\/code> pointer as its first argument.<\/p>\n
    What do you think? Could friend struct This<\/code> be a useful, low-overhead addition to the C++ refactoring toolbox?<\/p>\n","protected":false},"excerpt":{"rendered":"
    My day to day programming environment is C# + Visual Studio + ReSharper. I like to think I’m fairly productive in this setup and after years of experience have picked up a few fast and easy ways to refactor my… <\/p>\n","protected":false},"author":1,"featured_media":0,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[91,101],"tags":[],"class_list":["post-5380","post","type-post","status-publish","format-standard","hentry","category-design","category-native"],"_links":{"self":[{"href":"http:\/\/writeasync.net\/index.php?rest_route=\/wp\/v2\/posts\/5380","targetHints":{"allow":["GET"]}}],"collection":[{"href":"http:\/\/writeasync.net\/index.php?rest_route=\/wp\/v2\/posts"}],"about":[{"href":"http:\/\/writeasync.net\/index.php?rest_route=\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"http:\/\/writeasync.net\/index.php?rest_route=\/wp\/v2\/users\/1"}],"replies":[{"embeddable":true,"href":"http:\/\/writeasync.net\/index.php?rest_route=%2Fwp%2Fv2%2Fcomments&post=5380"}],"version-history":[{"count":6,"href":"http:\/\/writeasync.net\/index.php?rest_route=\/wp\/v2\/posts\/5380\/revisions"}],"predecessor-version":[{"id":5386,"href":"http:\/\/writeasync.net\/index.php?rest_route=\/wp\/v2\/posts\/5380\/revisions\/5386"}],"wp:attachment":[{"href":"http:\/\/writeasync.net\/index.php?rest_route=%2Fwp%2Fv2%2Fmedia&parent=5380"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"http:\/\/writeasync.net\/index.php?rest_route=%2Fwp%2Fv2%2Fcategories&post=5380"},{"taxonomy":"post_tag","embeddable":true,"href":"http:\/\/writeasync.net\/index.php?rest_route=%2Fwp%2Fv2%2Ftags&post=5380"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}