0

I am having some trouble with an unexpected output in the following simple code. The output works fine with the base class but I run into trouble with the derived class for some reason.

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


class vehicle {
public:
    string* name;
    vehicle() {}
    vehicle(const string& inputName) {name = new string(inputName);} //constructor
    virtual ~vehicle() {delete name; name = nullptr;} // always make base class destructor virtual
    //vehicle(const vehicle& rhs) {*this = rhs; cout << "copy called" << endl;} //copy constructor
    vehicle& operator=(const vehicle& rhs) {
        cout << "assignment called" << endl;
        if(this == &rhs){
            return *this;
        }
        name = new string(*rhs.name);
        return *this;
    } //assignment operator
    virtual string& getName() const {return *name; cout << "base" << endl;} //mark function const if we are not going to make any changes to the object
};

class car : public vehicle {
    string *title = new string;
public:
    car() : vehicle() {}
    car(const string& inputName) : vehicle(inputName) {*title = inputName;}
    virtual ~car() {}
    virtual string& getName() const {string temp = *title; return temp; cout << "derived" << endl;}
};
int main() {

    car test("honda");
    string temp;
    temp = test.getName();
    cout << temp;

    return 0;
}

I am intending to get the following output:

honda

But am getting:

~weird square boxes~

Edit:

What i'm really trying to accomplish is something like the following in the derived class:

virtual string& getName() const {return "Car: " + *name;} 

Before I get enflamed for using the heap for the string, please know that I am only experimenting here. It is my understanding that this should, in theory, work.

  • 1
    You're returning a reference to a temporary. Bad things will happen. – Stephen Newell May 10 '21 at 22:32
  • 1
    Why are `name` and `title` pointers? – Nathan Pierson May 10 '21 at 22:34
  • 1
    `string *title = new string;` drop the `*` and fix all uses of `title` that dereference it. It should not be a pointer – Ryan Haining May 10 '21 at 22:37
  • 3
    In general, pointer to `string` is not that useful. In it's most basic form, `string` is a pointer and a count and you gain nothing by adding another pointer into the mix. Dynamically allocating a string is a also a poor move. `string`s job is to take care of the nitty gritty of dynamic allocation for you. If you find yourself with a problem where you need to take managing the allocation back upon yourself, `string` probably isn't the right tool for the job. – user4581301 May 10 '21 at 22:46
  • 2
    C++ is not Java. Stop using the `new` keyword. – Ben Voigt May 10 '21 at 22:49
  • Please see edit – ilikemath3.14 May 11 '21 at 00:12
  • @ilikemath3.14 Your edit does nothing to reduce the concerns raised by others. You still have not demonstrated anything that benefits from (much less necessitates) using a dynamically-allocated `string` object. Plus you have demonstrated a greater need for `getName()` to avoid returning a reference to a string. – JaMiT May 11 '21 at 01:58
  • 1
    Does this answer your question? [C++ Returning reference to local variable](https://stackoverflow.com/questions/4643713/c-returning-reference-to-local-variable) – JaMiT May 11 '21 at 01:58
  • @JaMiT The suggestion of not returning by reference suffices. Thank you. – ilikemath3.14 May 11 '21 at 02:13

1 Answers1

4

This function

virtual string& getName() const {string temp = *title; return temp; cout << "derived" << endl;}

invokes undefined behavior because it returns a reference to a local object temp that will not be alive after exiting the function.

You could define the function like

virtual const string& getName() const {return *name;}

and

const string& getName() const override { return *title;}

And the statements after return statements do not have an effect.

Also other your code has drawbacks. For example the copy assignment operator produces a memory leak.

Or you need explicitly to define the destructor for the class car.

Pay attention to that there is no sense to declare data members name and title as pointers to objects of the type std::string. Instead declare data members of the type std::string.

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
  • Can you explain why the copy assignment operator produces a memory leak? Explicitly defining the copy constructor for car did not fix the issue: virtual ~car() {vehicle::~vehicle(); delete title; title = nullptr;} – ilikemath3.14 May 11 '21 at 00:08