Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

A better way to avoid public member invisibility and source code bloat/repetition with inherited class templates?

Tags:

c++

templates

Context:
Inheritance of protected and public class members is a fundamental concept of Object Oriented Programming. The trivial example below illustrates an often encountered situation in which the class CDerived inherits all public members of the class CBase and adds 1 additional function of its own without changing nor explicitly redeclaring nor redefining any of the public members of the CBase class.

#include <stdio.h>

class CBase
{
public:
    char Arr[32];

    int Fn1(void) {
        return Arr[1] ^ Arr[sizeof(Arr)-1];
    }

    int Fn2(void) {
        return Arr[2] ^ Arr[sizeof(Arr)-2];
    }
};


class CDerived : public CBase
{
public:  
    int FnSum(void) {
        return Fn1() + Fn2();
    }
};

int main(void)
{
    CDerived ddd;

    printf("%d\n", ddd.Fn1());
    printf("%d\n", ddd.Fn2());
    printf("%d\n", ddd.FnSum());

    return (int)ddd.Arr[0];
};

The code above compiles without problems on all major compilers.

However, if one wishes to "templatize" this code, e.g.: by parametrizing the size of the Arr array, then all public members of CBase class template become invisible to the CDerived class template on compilers that conform to the latest C++ standard.
Below is the problem code:

#include <stdio.h>

template <unsigned int BYTES>
class CBase
{
public:
    char Arr[BYTES];

    int Fn1(void) {
        return Arr[1] ^ Arr[sizeof(Arr)-1];
    }

    int Fn2(void) {
        return Arr[2] ^ Arr[sizeof(Arr)-2];
    }
};

template <unsigned int BYTES>
class CDerived : public CBase<BYTES>
{
public:

    int FnSum(void) {
        return Fn1() + Fn2() + Arr[0];  // ERRORs: identifiers "Fn1" and "Fn2" and "Arr" are NOT found !
    }
};    

int main(void)
{
    CDerived<32> ddd;

    printf("%d\n", ddd.Fn1());  //No error here
    printf("%d\n", ddd.Fn2());  //No error here
    printf("%d\n", ddd.FnSum());

    return (int)ddd.Arr[0];   //No error here
}

See:
MSVC v19.10: https://godbolt.org/g/eQKDhb
ICC v18.0.0: https://godbolt.org/g/vBBEQC
GCC v8.1: https://godbolt.org/g/GVkeDh

There are 4 solutions to this problem:

Solution #1: Prefix all of the references to members of the CBase class template (even the public ones), with CBase<BYTES>:: like this:

 int FnSum(void) {
        return CBase<BYTES>::Fn1() + CBase<BYTES>::Fn2() + CBase<BYTES>::Arr[0];  
 }

See:
MSVC v19.10: https://godbolt.org/g/48ZJrj
ICC v18.0.0: https://godbolt.org/g/BSPcSQ
GCC v8.1: https://godbolt.org/g/Vg4SZM

Solution #2: Prefix all of the references to members of the CBase class template (even the public ones), with this-> like this:

 int FnSum(void) {
        return this->Fn1() + this->Fn2() + this->Arr[0];  
 }

See:
MSVC v19.10: https://godbolt.org/g/oBs6ud
ICC v18.0.0: https://godbolt.org/g/CWgJWu
GCC v8.1: https://godbolt.org/g/Gwn2ch

Solution #3: Add one using statement inside the CDerived class template, for each member of the CBase (even a public one) that is referenced by the CDerived, like this:

using CBase<BYTES>::Arr;
using CBase<BYTES>::Fn1;
using CBase<BYTES>::Fn2; 

See:
MSVC v19.10: https://godbolt.org/g/gJT8cX
ICC v18.0.0: https://godbolt.org/g/1RK84A
GCC v8.1: https://godbolt.org/g/d8kjFh

Solution #4: Disable the strict conformance to the C++ standard by enabling the "permissive" mode in the compiler settings, like this:

For MSVC v19.10 remove the switch /permissive-, see: https://godbolt.org/g/Yxw89Y
For ICC v18.0.0 add the switch -fpermissive, see: https://godbolt.org/g/DwuTb4
For GCC v8.1 add the switch -fpermissive, see: https://godbolt.org/g/DHGBpW

MSVC NOTE: According to this article, by default the /permissive- option is set in new projects created by Visual Studio 2017 v15.5 (MSVC compiler v19.11) and later versions. It is not set by default in earlier versions, ...including the latest Godbolt.org's Compiler Explorer MSVC version v19.10.

GCC NOTE: Even with the -fpermissive compiler switch, the GCC v8.1 compiler still needs the using CBase<BYTES>::Arr; statement inside the CDerived class (...or one of the other solutions) in order to make the public Arr array visible inside the CDerived class template ...but it does not need anything extra to make the Fn1() and Fn2() functions visible.

MSVC Non-Solution: According to this article and this article, the compilation error in MSVC comes from the Two-Phase Name Lookup being enabled by the conformance to the C++ standard mode ( the /permissive- option).
Also, according to the former article: "The /permissive- option implicitly sets the conforming two-phase lookup compiler behavior, but it can be overridden by using /Zc:twoPhase- switch".
However adding the two compiler switches /permissive- /Zc:twoPhase- does not cause the "templated" problem code to compile in MSVC v19.14, without the additions described in Solution #1 or #2 or #3.

MSVC v19.14: https://godbolt.org/z/BJlyA8

See this entry for more details.

Problems with above Solutions:
Solution #4 is not portable and breaks away from the C++ standard. It is also a GLOBAL solution (global switch) to a local problem - usually a bad idea. A compiler switch that affects only a portion of the code (e.g. #pragma NOtwoPhase) does not exist.
Solution #1 has an unintended side-effect of suppressing virtual calls, thus it is not applicable in general case.
Both solutions #1 and #2 require many verbose additions to the code. This leads to a source code bloat that does not add any new functionality. For example if the CDerived class template adds only 2 functions to a CBase class that contains 5 public functions and 1 member variable, which are referenced multiple times in CDerived, the Solution #1 requires 14 verbose code alterations/additions in the derived class, which look like this:

    #include <stdio.h> 

    template <unsigned int BYTES>
    class CBase
    {
    public:
        char Arr[BYTES];

        CBase() {
            for (size_t i=1; i<sizeof(Arr); i++)
            Arr[i] = Arr[i-1]+(char)i;
        }   

        int Fn1(void) {
            return Arr[1] ^ Arr[sizeof(Arr)-1];
        }

        int Fn2(void) {
            return Arr[2] ^ Arr[sizeof(Arr) - 2];
        }

        int Fn3(void) {
            return Arr[3] ^ Arr[sizeof(Arr) - 3];
        }

        int Fn4(void) {
            return Arr[4] ^ Arr[sizeof(Arr) - 4];
        }

        int Fn5(void) {
            return Arr[5] ^ Arr[sizeof(Arr) - 5];
        }
    };


    template <unsigned int BYTES>
    class CDerived : public CBase<BYTES>
    {
    public:

        int FnSum(void) {
            return CBase<BYTES>::Fn1() +
            CBase<BYTES>::Fn2() + 
            CBase<BYTES>::Fn3() + 
            CBase<BYTES>::Fn4() + 
            CBase<BYTES>::Fn5() + 
            CBase<BYTES>::Arr[0] +
            CBase<BYTES>::Arr[1] +
            CBase<BYTES>::Arr[2];
        }

        int FnProduct(void) {
            return CBase<BYTES>::Fn1() * 
            CBase<BYTES>::Fn2() * 
            CBase<BYTES>::Fn3() * 
            CBase<BYTES>::Fn4() * 
            CBase<BYTES>::Fn5() * 
            CBase<BYTES>::Arr[0] *
            CBase<BYTES>::Arr[1] *
            CBase<BYTES>::Arr[2];
        }  
    };

    int main(void)
    {
        CDerived<32> ddd;

        printf("%d\n", ddd.FnSum());
        printf("%d\n", ddd.FnProduct());

        return (int)ddd.Arr[0];
    }

In real life the Base class template might contain ~50 functions and many variables which are referenced multiple times in the Derived class template, which necessitate 100s of such repetitive edits !
There must be a better way...

Solution #3 requires less work because it does not require finding and prefixing EVERY REFERENCE to the CBase member in the CDerived's code. The CBase members, that are used by CDerived, need to be "re-declared" with a using statement only once, regardless how many times these members are used/referenced in the CDerived's code. This saves a lot of mindless searching and typing.

Unfortunately a blanket statement like using CBase<BYTES>::* which makes all of the protected and public members visible in the derived class template, does not exist.

QUESTION:
Is there a less verbose portable solution to this problem ? e.g. Solution #5...

like image 397
George Robinson Avatar asked May 14 '18 00:05

George Robinson


3 Answers

At the risk of getting downvoted, I'm going to go on a limb and intentionally not answer your question. In fact I am going to do the opposite and say that the whole endeavor is misguided from the get-go.

The type of scenarios like you describe, where a child class invokes methods or refer to members of its parent class is, with the exception of a few specific cases, considered bad code. It's called inherit-to-extend if you want to read more on that anti-pattern. Good SO answer as an intro on the subject

Ok, well it's not so much bad code, as it is a code smell: a vague indication that something is not quite right in the fundamental design of the code.

Code smells are ok, you don't necessarilly have to go out of your way to avoid every single one of them, and the pattern you described might genuinely be the right thing to do in your case. However, it would be naughty code, that deserves a big comment block to explain why it's ok in this instance.

Jumping through hoops to make it easier to write naughty code is just a bad idea.

like image 58
Frank Avatar answered Sep 22 '22 07:09

Frank


Use macros to simplify Solution #3 somewhat. Boost is not strictly necessary, but makes life easier.

#include <boost/preprocessor.hpp>

#define USING_ONE(r, base, member)              \
    using base::member;

#define USING_ALL(base, ...)                    \
    BOOST_PP_SEQ_FOR_EACH(                      \
        USING_ONE, base,                        \
        BOOST_PP_VARIADIC_TO_SEQ(__VA_ARGS__)   \
    )

// Near CBase<BYTES>
#define USING_CBASE(param) USING_ALL(CBase<param>, Arr, Fn1, Fn2, Fn3, Fn4, Fn5)

// In CDerived<BYTES>, in a `public:` section
USING_CBASE(BYTES);
like image 6
o11c Avatar answered Nov 05 '22 17:11

o11c


I was suffering from the same issue. I went through All Solutions posted by George Robinson. While I found Solution #2 and Solution #3 the most helpful and concise with the least CodeSmell. I try to avoid using any naked pointers and I rarely use the this Keyword and hence didn't want to use Solution #2.

However while I was writing the code for Template Inheritance involving some fairly complex templates.

template<typename T1, typename T2>
class Runnable{
  // Class Code
};

template<typename T1, typename T2, typename T3, typename T4>
class Task: Runnable<vector<pair<T1,T2>>,vector<pair<T3,T4>>> {
  // Derived Class Code
};

clearly applying Solution#3 was not favorable to me and hence. I found a work around that to me was cleared and made the code a bit nicer as well.

template<typename T1, typename T2, typename T3, typename T4>
class Outer{
  public: 
    using Runnable_ = Runnable<vector<pair<T1,T2>>,vector<pair<T3,T4>>>;

    class Task: Runnable_{
      // Inner Class Code
      // Need to use only Runnable_ instead of Runnable<vector<pair<T1,T2>>,vector<pair<T3,T4>>>
      using Runnable_ run;
      // Remaining Class Code

    };

};

Outer<int, int, int, int>::Task task;

While this isn't as efficient as the Boost Solution. It helps a lot while writing complicated template classes without using this->.

When writing nested template classes the Outer class is usually necessary and thus the overhead in terms of code is much lower.

like image 1
user12984287 Avatar answered Nov 05 '22 18:11

user12984287