0

Can someone explain if the following approach is correct for many layers of inheritance? I'm getting a bit confused...

say I have an enum class defined in a namespace that is imported and being used in all these files:

{
    RED = 1,
    BLUE = 2,
    GREEN = 3
};

say this is the parent file:

parent.hpp

class Parent {
  public:
  Parent (const std::string &name = "", Color color = Color::RED)
   : a_name(name)
   , a_color(color) {};

   Color getcolor() const { return a_color; }

   protected:
   std::string a_name;
   Color a_color;

}

Then I have a child class that inherits the parameters and fields of parent, but has its own new parameter.

child.hpp

class Child : public Parent
{
  public:
  Child(std::string name, Color color, int number = 0);
  protected:
  int number;
}

child.cpp

Child::Child (std::string name, Color color, int number)
  : parent(name, color)
  , a_number(number) {};

and then grandchildren who inherit from child

Grandchild.hpp

class Grandchild : public Child
{
  public:
  Grandchild(Std::string name, Color color, int number, int age = 0);
  private:
  int a_age;
}

Grandchild.cpp

Grandchild::Grandchild (Std::string name, Color color = Color::RED, int number, int age)
  : Child (name, color, number)
  , a_age(age)

Let's say I want to define a color for each grandchild, would I put it inside the constructor like above? Or would I do something like this:

Grandchild::Grandchild (Std::string name, int number, int age)
  : Child (name, Color::RED, number)
  , a_age(age)

Ruo
  • 77
  • 7
  • 1
    By providing pseudo code instead of proper definitions you make it unnecessarily cumbersome to help you. – Ted Lyngmo Jun 27 '23 at 20:09
  • 2
    `Grandchild::Grandchild (Std::string name, Color color = Color::RED, int number, int age)` `color` has a default value. 1) the caller could change it, something you cannot do with the other code sample, and 2) you can't have a non-defaulted parameter after a defaulted parameter. – user4581301 Jun 27 '23 at 20:10
  • Regarding 1, what is your intent? `Grandchildren` are always red? If so that's code sample 2. If you want red unless otherwise specified that's sample one with `color` moved to be the final parameter. – user4581301 Jun 27 '23 at 20:13
  • @user4581301 The intent is that grandchildren should be red, unless otherwise specified. What do you mean when you say that color should move to the final parameter? – Ruo Jun 27 '23 at 20:18
  • Default parameters always need to be last so that the compiler knows which parameters have been left out. If you `Grandchild G{"Syd", 42, 24)`, the compiler will try to stuff 42 into `color` and fail. It's not smart enough to recognize that the type's not correct and move on to the next parameter. – user4581301 Jun 27 '23 at 20:24
  • This code does not compile. Is it supposed to compile? Is it supposed to be pseudo-code? – Eljay Jun 27 '23 at 20:40
  • Aside from the syntax errors (or, pseudo-code?), there is a serious problem with passing a `std::string` parameter by **value** multiple times. Since C++11 (more than a decade ago!), the "correct" way of handling this is to take the string parameter by value and then **move** it into the member field. The by-value parameter taken by the `Grandchild` constructor should be `std::move`d into the `Child` constructor's by-value parameter, which should then be `std::move`d into the `Parent` constructor, which should also take a by-value parameter, and then `std::move`d into the member field. – monkey0506 Jun 27 '23 at 21:02
  • Just to clarify, the `std::string name` parameter in the `Grandchild` constructor is an **lvalue**. When you then pass `name` as the parameter to the `Child` constructor, this **creates a copy** of the string. If you instead use `std::move(name)`, this casts the parameter to an **rvalue reference**, which will invoke the *move constructor* of `std::string` rather than the *copy constructor*. Moving the string from `Grandchild` to `Child` to `Parent` and then into `Parent::a_name` is significantly faster and more memory efficient than performing 3-4 copies of the string. – monkey0506 Jun 27 '23 at 21:22
  • *What do you mean when you say that color should move to the final parameter?* It means this code does not compile. – Eljay Jun 27 '23 at 22:09

2 Answers2

0

You could use:

GrandChild( std::string name, int number, int age = 0 )
: Child( name, Color::RED, number ), a_age( age )
{}

The alternative syntax you have:

Grandchild::Grandchild (Std::string name, Color color = Color::RED, int number, int age)
: Child (name, color, number), a_age(age)

isn't legal because you're truing to use a default argument in the definition, not the declaration. If grandchildren weren't all red but you still wanted a default color, you could use:

GrandChild( std::string name, int number, Color color = Color::RED, int age = 0 )
: Child(name, color, number ), a_age( age )
{}

Note that we had to switch the order of color and number because otherwise color couldn't have a default argument (number came later and had no default).

user1806566
  • 1,078
  • 6
  • 18
  • I missed something above: a default parameter is applied at the call site, so the default needs to be declared in the function declaration, that's in the header, rather than the definition, which will be invisible to most callers because it's in the source file. – user4581301 Jun 27 '23 at 20:19
  • Yes, that's true. In my snippets, I'm assuming they're declarations and inline definitions (i.e., what's in the class declaration in the header file). – user1806566 Jun 27 '23 at 20:55
0

The best syntax probably is the following.

GrandChild(std::string, int number, int age = 0) :
Child(std::move(name), Color::RED, number),
a_age(age)
{}

Alternatively you could also create a template based grand child.

template <int color>
class GrandChild {
...
GrandChild(std::string name, int number, int age = 0) :
Child(std::move(name), color, number),
a_age(age)
...
};
using RedGrandChild = GrandChild<Color::RED>;
using GreenGrandChild = GrandChild<Color::GREEN>;
using BlueGrandChild = GrandChild<Color::BLUE

This depends on what you are using it for.

user7934593
  • 120
  • 6
  • 1
    Because the string is ultimately being stored in a member field (`Parent::a_name`), it makes much more sense to accept the `std::string` parameters in each constructor by-value and `std::move` them. This is the only way to accept temporary `std::string` objects (`std::string&&`) without multiple explicit overloads of each constructor, and in both cases only a single copy of the string is done (which is necessary for the by-value member field anyway). – monkey0506 Jun 27 '23 at 21:13
  • Also, passing primitive types (like `int`) by reference (`const` or non-`const`) [is generally bad practice](https://stackoverflow.com/questions/14013139/is-it-counter-productive-to-pass-primitive-types-by-reference). – monkey0506 Jun 27 '23 at 21:16
  • @monkey0506 Thanks I did not know that! – user7934593 Jun 27 '23 at 21:23
  • 1
    Actually, I need to correct myself in one point on the reference to primitives comment. Passing primitives by non-`const` reference is fine if you intend to change the value. Passing primitives by `const` reference is considered bad practice because it's typically faster to just copy the immutable parameter values in that case. – monkey0506 Jun 28 '23 at 22:20