-4

i have a problem with my example code.

The task is to make a class with at least one dynamic member, to create some objects, print something out and destroy them at the end. I've written my code, it compiles without any errors or warnings, but it don't print anything(I'm using Microsoft CL-Compiler of VS 2017 Community Edition).

Can someone give me a hint, what i'm doing wrong?

dyn.h:

#include <iostream>
#include <string>

#ifndef __CLASS_DYN__
#define __CLASS_DYN__

class dyn{

    private: std::string Name;

    private: int* Age;

    public: dyn(std::string, int);
    public: dyn(const dyn&);

    public: ~dyn();

    public: std::string toString();

};

#endif

dyn.cpp

#include "dyn.h"

dyn::dyn(std::string Name, int Age){
    this->Name = Name;
    *this->Age = Age;
}

dyn::dyn(const dyn& a){
    this->Name = a.Name;
    *this->Age = *a.Age;
}

dyn::~dyn(){
    delete this->Age;
}

std::string dyn::toString(){
    std::string tmp = "Name of Person " + this->Name;
    return (tmp);
}

main.cpp

#include <iostream>
#include "dyn.h"

int main(){

    dyn* Person1 = new dyn{"Mike", 38};
    dyn* Person2 = new dyn{"Thomas", 20};
    dyn* Person3 = Person1;

    std::cout << Person1->toString() << std::endl;
    std::cout << Person2->toString() << std::endl;
    std::cout << Person3->toString() << std::endl;

    delete Person1;
    delete Person2;
    delete Person3;

    return 0;
}
Ronny Kaufmann
  • 408
  • 2
  • 7
  • 3
    `*this->Age = Age;` Here `Age` doesn't point to anything, and yet you're trying to dereference it. Use `this->Age = new int(Age);`. Same for the copy constructor. Also you forgot to write copy assignment operator. Also note that you have to remove `delete Person1;` OR `delete Person3;` from `main`, since they point to the same object. Also what's the point of writing `public:` or `private:` before *each* member? – HolyBlackCat Jun 02 '19 at 20:27
  • Thank you, that do the trick. The public-private-thing: I'm usually coding in c# but want to learn c++. You're complete right, in c++ it isn't needed, but in c# i learned it this way (it will take some time to accustom myself ;)) – Ronny Kaufmann Jun 02 '19 at 20:47
  • 2
    This doesn't address the question, but names that contain two consecutive underscores (`__CLASS_DYN__`) and names that begin with an underscore followed by a capital letter are reserved for use by the implementation. Don't define names like that in your code. – Pete Becker Jun 02 '19 at 22:11

3 Answers3

3

It's in both of the lines where you assign to Age. In these cases, since Age is declared as a int *, *this->Age is not this->Age itself, but the the value at the memory address that this->Age points to. Further, since these are both in the constructors, this->Age does not yet hold a valid address, so trying to assign a value to it will result in undefined behaviors.

The proper thing to do is to ensure that this->Age has a valid address before assignment by allocating memory as follows:

dyn::dyn(std::string Name, int Age){
    this->Name = Name;
    this->Age = new int();
    *this->Age = Age;
}

dyn::dyn(const dyn& a){
    this->Name = a.Name;
    this->Age = new int();
    *this->Age = *a.Age;
}

And fortunately, you're already deleting Age in the destructor, so no modification is needed there.

tdk001
  • 862
  • 1
  • 7
  • 16
1

Thx. to @HolyBlackCat:

I changed my ctor::

dyn::dyn(std::string Name, int Age){
    this->Name = Name;
    this->Age = new int(Age);
}

and

dyn::dyn(const dyn& a){
    this->Name = a.Name;
    this->Age = new int{*a.Age};
}

Now it works as intended :)

Ronny Kaufmann
  • 408
  • 2
  • 7
1

but want to learn c++

So you while you already have an appropriate answer to the problem itself, I might give you some additional hints...

At first, well, you discovered already, you can pass the argument to the pointer's constructor itself:

this->Age = new int(Age);
this->Age = new int{*a.Age};

Solely: there are two ways of calling a constructor, the classic one with parentheses and the new, uniform initialisation via braces. While the latter showed some good intent, I personally consider it broken and don't use it in my own code (favourite example: std::vector<int>{1, 2, 3} calls the std::initializer_list constructor, std::vector<int>{7} calls classic constructor creating a vector with seven elements; compare to: std::vector<int>(7) and std::vector<int>({7}), the latter explicitly calling the std::initializer_list constructor). Well, you might want to dig a bit further into the topic and decide yourself if you want to follow me or the ones in favour of UI, but whatever you opt for, you should use it consistently and not switch in between the two...

Then get used to use the constructors initialiser list (not to be confused with std::initializer_list!):

dyn::dyn(std::string Name, int Age)
    : Name(Name)
//          ^ function argument
//      ^ member
{
    // ...
}

Well, it even is possible to use the same identifiers (position makes clear which one is which), still you might prefer to use differing ones. There isn't much difference for the native types (int, double, pointers, ...), for complex types, though, the matter changes:

dyn::dyn(std::string Name, int Age)
    // default constructor for Name is  s t i l l  called!
{
    this->Name = Name; // and now you do the assignment!
}

Both of default constructing and assignment might cost more or less, in any case, direct initialisation, as occurs by using the initialiser list as shown before, is more efficient. Additionally, references and non-default-constructible types only can be initialised this way.

Then argument type: C# decides by type if an argument is passed by value or by reference; in C++, you can pass any type by reference or by value (or, third option, via pointer), but you need to be explicit about!

dyn::dyn(std::string   Name, int Age) // by value (i. e. you make a  c o p y  of!)
dyn::dyn(std::string&  Name, int Age) // by reference
dyn::dyn(std::string*  Name, int Age) // by pointer
dyn::dyn(std::string&& Name, int Age) // for completeness: r-value reference

Last one is a new concept allowing move semantics (i. e. moving the contents from one object into another one). There's a good tutorial already, so I won't address further.

However, by accepting a reference, you avoid one unnecessary copy (and as you do not intend to change the argument, it should be const):

dyn::dyn(std::string const& Name, int Age);

Then have a look at the rule of three. It is true that you get a default assignment operator defined by C++, but it will simply do the following:

dyn::operator=(dyn const& other)
{
    Name = other.Name;
    Age = other.Age;    // now both objects will point to the same address
                        // and you'll get a double deletion error!
}

OK, there's move-assignment defined, too, but that won't change anything for pointers. With move semantics, the Ro3 got extended to the rule of five; while the former is (still) mandatory, the latter isn't, but you are missing a great optimisation possibility if you don't follow it...

Modern C++ way dealing with dynamically allocated memory is by use of smart pointers, there are std::unique_ptr,std::shared_ptr and std::weak_ptr (the latter one far less commonly used).

class dyn
{
private:
    std::unique_ptr<int> Age;
public:
    dyn(int Age) : Age(std::make_unique<int>(Age)) { }
};

And guess what: now default generated destructor, move and copy constructors and assignment all are already fine, you don't need to implement any of these explicitly...

return is not a function, so you shouldn't use parentheses around the returned value. In C++, this even can have concrete effects:

  1. It prevents copy elision on return values.
  2. In a rather specific scenario you even can create a dangling reference:
    decltype(auto) f() { int n; return (n); } // returns a reference to n!

Finally, one last note about naming conventions. Usually, class identifiers have a leading capitlal letter, where variables (members, globals and local ones) start with a small letter. Function identifiers are ambigous, there are some favouring capital letters (a convention mainly coming from Microsoft), others prefer the initial minuscules still today (which is the far older convention and still the more commonly followed one). For functions, you should opt for one or the other, but in any case, follow it consistently.

Aconcagua
  • 19,952
  • 4
  • 31
  • 51