0

sorry if this is an obvious one but I've searched around and I'm still unclear about how to solve this issue. Here is my code:

#include <iostream>
#include <string>
using namespace std;

class PermMagnet {
public:
    string name;
    int ac_rating;
    int dc_rating;
    int mass_kg;
    int age;
    PermMagnet(){
        // default constructor
        name = "";
        ac_rating = 0; dc_rating = 0;
        mass_kg = 0; age = 0;
    }
    PermMagnet(string c_name, int c_ac_rating, int c_dc_rating, int c_mass_kg, int c_age){
        // parameterised constructor
        name = c_name;
        ac_rating = c_ac_rating;
        dc_rating = c_dc_rating;
        mass_kg = c_mass_kg;
        age = c_age;
    }
    string get_owner(){
        return owner;
    }
    string get_classifier(){
        return classifier;
    }
    int get_coil_count(){
        return coil_num;
    }
protected:
    string location = "facility hall";
private:
    string owner = "Unspecified Staff";
    string classifier = "MAG-DP-";
    const int coil_num = 2;
};

class ElecMagnet : public PermMagnet {
public:
    // inherit base class constructors
    using PermMagnet::PermMagnet;

    string get_location(){
        return location;
    }

private:
    string owner = "Specified Staff";
    string classifier = "MAG-QD-";
    const int coil_num = 4;
};

int main() {

    // Create object using default constructor
    PermMagnet perm1;
    cout << "'perm1' age: " << perm1.age << endl;

    // Create object using parameterised constructor
    PermMagnet perm2("PermMagnet 2", 380, 400, 1500, 35);
    cout << "'perm2' age: " << perm2.age << " | 'perm2' name: " << perm2.name << endl;
    cout << "Owner of 'perm2': " << perm2.get_owner() << endl;
    cout << "Upper current bound of 'perm2': " << perm2.get_current_limit("upper") << "A" << endl;
    cout << "Number of coils in 'perm2': " << perm2.get_coil_count() << endl << endl;

    // Create a ElecMagnet (derived class) object
    ElecMagnet elec1("ElecMagnet 1", 170, 200, 850, 27);
    cout << elec1.get_classifier() << endl;
    cout << elec1.get_coil_count() << endl;

    return 0;
}

The output of this code is:

'perm1' age: 0
'perm2' age: 35 | 'perm2' name: PermMaget 2
Owner of 'perm2': Unspecified Staff
Upper current bound of 'perm2': 780A
Number of coils in 'perm2': 2

MAG-DP-
2

Process finished with exit code 0

As you can see, I want "owner", "classifier" and "coil_num" to be private members that cannot be changed by the user. These also vary depending on the class in question.

The problem:

The problem is the last two lines of the output. When the derived class (ElecMagnet) inherits the public functions that return these members, it returns the members of the base class; not it's own. You can see this because it returns the "classifier" and "coil_num" of the PermMagnet class.

Does anyone know why the derived class is behaving this way? Shouldn't it access its own private members rather than the base?

Cato
  • 29
  • 5
  • You cannot "override" member variables, only member functions, and the latter requires you to specify them as `virtual` anyway. Here, the base function only knows about its members with those names. The derived ones with the same names _hide_ them, but don't _override_ them. Rethink your design. For instance, each class could have `virtual` functions to return those strings, which would then return a string depending on the concrete class type (even if called through a base reference/pointer) - no storage of them as members needed. – underscore_d Dec 23 '20 at 15:00
  • 4
    Is there a reason you need two different `owner` variables (with the same variable name!) for each `ElecMagnet` object? Is there a reason you can't just use the base class's `owner`? – scohe001 Dec 23 '20 at 15:01
  • 2
    You invoke the public base class public method which accesses base class private members. Why would you expect a base class method to be able to access derived class members? Why not make those base members which you duplicate in derived class protected instead of private? That way reduces duplication and allows derived class access. – mathematician1975 Dec 23 '20 at 15:02
  • 1
    Some bad practices and lack of understanding. Repeating variables should have been a red flag because then literally what's the point of inheritance? – sweenish Dec 23 '20 at 15:04
  • I thought the purpose of a private member was that it could not be accessed outside of the class it is defined in. – Cato Dec 23 '20 at 15:05
  • 1
    Inheritance uses an "is a" relationship. Wherever you learned about this should have mentioned that pretty close to the beginning. – sweenish Dec 23 '20 at 15:06

3 Answers3

0

Private members can only be accessed by member functions of the same class or friends. C++ does not behave like, for example, Python, where the search for variables is performed dynamically depending on the current pointer this (self).

alex_noname
  • 26,459
  • 5
  • 69
  • 86
0

This happens because you're still calling the method of the base class, which cannot know about the member variables of the derived class and thus will not use them. To archive what you want, you need to override get_classifier() in the derived class so that calling it via a ElecMagnet-reference will call the derived method. You can optionally make the method virtual, to even get the derived version called, when invoking it on a ElecMagnet-value via a PermMagnet-reference. (Making classes designed for inheritance virtual is generally recommended.)

Alternatively, depending on your use case, instead of using the base constructors, you could write explicit constructors that call a (possible protected) base constructor with an appropriate value for classifier. This would save you the additional member variables in the derived class entirely.

Reizo
  • 1,374
  • 12
  • 17
  • Thanks Reizo, much appreciated. Your first paragraph solved my problem but I will try to implement your second suggestion too as it sounds more efficient. – Cato Dec 23 '20 at 15:19
  • You're welcome. If this answer solved your problem, consider accepting it. Otherwise, if you need some help for my alternative approach, feel free to ask and I will extend my answer :) – Reizo Dec 23 '20 at 15:30
0

Inheritance uses an "is a" relationship. Meaning, you should be able to say that a child object "is a" base object. If you cannot say this, you shouldn't be using inheritance.

What this means for the building of a derived object is that it is a composite, containing your base object, and whatever extra bits have been added through inheritance.

You called a base object function, which means it will access base object data. Repeating the data in the derived class obviously doesn't work, and is a bad practice. If you need the same data, use it and don't repeat it.

You do this by having your derived class constructors always call base class constructors first in the initialization section. You don't use the initialization section at all, this can lead to a lot of inefficiencies and eventually errors.

Random bits: All class data should be private. The protected: section of a class holds whatever you want a child to have public access to, but don't want to make fully public. If you don't want getters to be publicly available, they can go there. I placed a couple constructors that exist solely for the sake of ElecMagnet here. using namespace std; is bad practice.

The code:

#include <iostream>
#include <string>

class PermMagnet {
 public:
  PermMagnet() = default;
  PermMagnet(std::string c_name, int c_ac_rating, int c_dc_rating,
             int c_mass_kg, int c_age)
      : name(c_name),
        ac_rating(c_ac_rating),
        dc_rating(c_dc_rating),
        mass_kg(c_mass_kg),
        age(c_age) {}  // Use the initialization section

  std::string get_owner() const { return owner; }  // Mark getters as const

  std::string get_classifier() const { return classifier; }

  int get_coil_count() const { return coil_num; }

  std::string get_location() const { return location; }

  int get_age() const { return age; }

  std::string get_name() const { return name; }

 protected:
  PermMagnet(std::string owner, std::string classifier, int coilNum)
      : owner(owner), classifier(classifier), coil_num(coilNum) {}

  PermMagnet(std::string name, int ac_rating, int dc_rating, int mass_kg,
             int age, std::string owner, std::string classifier, int coilNum)
      : name(name),
        ac_rating(ac_rating),
        dc_rating(dc_rating),
        mass_kg(mass_kg),
        age(age),
        owner(owner),
        classifier(classifier),
        coil_num(coilNum) {}

 private:
  std::string owner = "Unspecified Staff";
  std::string classifier = "MAG-DP-";
  const int coil_num = 2;  // const probably unnecessary here, but left it
  std::string location = "facility hall";
  std::string name = "";
  int ac_rating = 0;
  int dc_rating = 0;
  int mass_kg = 0;
  int age = 0;
};

class ElecMagnet : public PermMagnet {
 public:
  ElecMagnet() : PermMagnet("Specified Staff", "MAG-QD-", 4) {}

  ElecMagnet(std::string name, int ac_rating, int dc_rating, int mass_kg,
             int age)
      : PermMagnet(name, ac_rating, dc_rating, mass_kg, age, "Specified Staff",
                   "MAG-QD-", 4) {}

  // NO NEED FOR REPETITIVE PRIVATE SECTION
};

int main() {
  // Create object using default constructor
  PermMagnet perm1;
  std::cout << "'perm1' age: " << perm1.get_age()
            << '\n';  // Prefer '\n' over std::endl

  // Create object using parameterised constructor
  PermMagnet perm2("PermMagnet 2", 380, 400, 1500, 35);
  std::cout << "'perm2' age: " << perm2.get_age()
            << " | 'perm2' name: " << perm2.get_name() << '\n';
  std::cout << "Owner of 'perm2': " << perm2.get_owner() << '\n';
  std::cout << "Number of coils in 'perm2': " << perm2.get_coil_count()
            << "\n\n";

  // Create a ElecMagnet (derived class) object
  ElecMagnet elec1("ElecMagnet 1", 170, 200, 850, 27);
  std::cout << elec1.get_classifier() << '\n';
  std::cout << elec1.get_coil_count() << '\n';

  return 0;
}

Note that the ElecMagnet class is currently now just a couple constructors, and your main function behaves the way that's expected.

sweenish
  • 4,793
  • 3
  • 12
  • 23
  • Sweenish.. thank you for taking the time to refactor the code! As you can tell I am a beginner so explaining the best practices is much appreciated. Big thanks! – Cato Dec 23 '20 at 16:04
  • Also, when you say "all class data should be private", why do you say that? If you can explain the reason that would be appreciated. – Cato Dec 23 '20 at 16:11
  • Encapsulation is the principle at play. If the data can be public and freely manipulated, you want a `struct`. Otherwise the data should be private to mainly stop it from being freely manipulated. It's the big reason for having a class at all. We guarantee the class invariant (an object, through its data, is always in a valid state). Because a variable like `age` probably shouldn't be able to hold *any* value, we make it private and control how it's updated. – sweenish Dec 23 '20 at 16:22
  • Gotcha! Thanks very much. – Cato Dec 23 '20 at 17:26