0

I am a C++ newbie and I just started with class inheritance. I have previously had a bit of C++ experience with pointers and variables but don't remember much.

Here is my problem.

So I have 2 Classes, one base and one derivied

Base.h

#ifndef BASE_H
#define BASE_H

class Base {
protected:
    int* value_;
public:
    int* GetValue() { return value_; }
    void SetValue(int* value) { value_ = value; }
    Base() {
        int tmp = 5;
        SetValue(&tmp);
    }
};

#endif

Derived.h

#ifndef DERIVED_H
#define DERIVED_H

#include "Base.h"

class Derived : public Base {
public:
    Derived() { Base(); }
};

#endif

main.cpp

#include <iostream>

#include "base.h"
#include "derived.h"

int main() {
    Derived derived = new Derived();
    std::cout << derived->GetValue() << std::endl;
    return 0;
}

Console output

0

What am I doing wrong? The console output should be 5, right?

  • 2
    In the Base constructor, "tmp" is a local variable. This means that it goes out of scope when the constructor exits. After that, the data member "value" is pointing at garbage. – user888379 May 16 '23 at 20:17
  • 1
    `Derived derived = new Derived();` -- This is a red flag that you are using techniques from other languages to write C++ code. This could simply have been `Derived derived{};` -- There is no need to use `new` to create objects in C++. – PaulMcKenzie May 16 '23 at 20:26
  • 1
    `Derived() { Base(); }` does not do what you think it does. It needs to be `Derived() : Base() { }` instead, which can then be simplified to `Derived() { }` since base class *default* constructors are invoked implicitly. In which case, you can just remove your derived constructor entirely and let the compiler [implement it implicitly](https://en.cppreference.com/w/cpp/language/default_constructor#Implicitly-declared_default_constructor) for you. – Remy Lebeau May 16 '23 at 20:40
  • Header filenames are also case-sensitive, e.g. `Base.h` is not `base.h`. Why are you using pointers at all and why are you using `tmp`? Why not simply provide a constructor with a default to allow 0 or 1 parameters? E.g. `Base (int value = 5) { value_ = value; }` and forget about the pointers. Use the `'.'` operator for access. – David C. Rankin May 16 '23 at 21:56
  • 1
    @DavidC.Rankin — case sensitivity for header file names is [implementation defined](https://stackoverflow.com/questions/9450073/case-sensitivity-in-c-header-files). – Pete Becker May 17 '23 at 00:41
  • @PeteBecker so it is. On Linux, that's never a guessing game. Apparently on windows anything goes. Unfortunately, the answers in the linked question are not superb, the only definitive information coming from the comment below the accepted answer by paxdiablo. – David C. Rankin May 17 '23 at 03:54
  • @DavidC.Rankin — it’s up to the **compiler** to determine what a header name refers to. There’s nothing that prevents an implementation from using case-insensitive names on Linux. The OS has nothing to say about it. – Pete Becker May 17 '23 at 16:23
  • @PeteBecker - except it still has to be able to find the file on the filesystem... – David C. Rankin May 17 '23 at 19:13

3 Answers3

2

Your problem is that you are using pointers when you don't need pointers, and on top of that you are using pointers incorrectly. In particular, you are setting value_ to point to tmp, but tmp is a local variable that gets destroyed as soon as the Base() constructor returns, so value_ is left as a dangling pointer, and when you try to access it later on via GetValue(), you invoke undefined behavior.

You should change int* to int everywhere in your code, and don't use the new operator either (you don't need it in this example, and using it is causing your program to leak memory).

Jeremy Friesner
  • 70,199
  • 15
  • 131
  • 234
  • 1
    Maybe worth mentioning that `Derived() { Base(); }` should be `Derived() : Base() {}` too if OP wants to be explicit about the base class initialization for some reason. – Ted Lyngmo May 16 '23 at 20:59
0

To call the parent constructor from child constructor it needs to be done in the initializer list. for example:

class A;
class B : A
{
  B() : A() // <-- here
  {
  }
}

So your child class should be like:

class Derived : public Base {
public:
    Derived() : Base() {  }
};
Bader
  • 46
  • 3
  • You're right, and the OP should definetly update their code, but it doesn't explain the behaviour the question asks about... – fabian May 16 '23 at 20:26
  • The version of `Derived::Derived()` in the original code **does* call the base constructor. The compiler generates a call to the default constructor if there isn't a constructor call in the initializer list. Simply removing `Base();` from the constructor removes the wasted code. `Derived() { }` is sufficient. (Yes, some folks would make the base constructor call explicit anyway, on the theory that writing more code is better than writing less code) – Pete Becker May 16 '23 at 20:35
0

Nothing really wrong with your inheritance, basically an error in your assignment

base.h

#ifndef BASE_H
#define BASE_H

class Base {
protected:
    int value;
public:
    int GetValue() { return this->value; }    
    Base(int value) {
        this->value = value;        
    }
};

#endif

derived.h

#ifndef DERIVED_H
#define DERIVED_H

#include "base.h"

class Derived : public Base {
public:
    Derived(int value):  Base(value){};
};

#endif

and called like

  auto derived = new Derived(7);
  std::cout << derived->GetValue() << std::endl;
  delete derived;
  • The base constructor should directly initialize `value` rather than default-initializing it and immediately overwriting the default value. `Base(int value) : value(value) {}`. And in `GetValue()`, simply `return value;` is sufficient; `this->` isn't needed. – Pete Becker May 16 '23 at 20:38
  • i like to use this just to be more explicit, but yes it's unnecessary, if we are going to be picky i should also removed the new, but the question was about inheritance, so went for that. – Guillermo Gutiérrez May 16 '23 at 20:42