0

I would like to make a vector of objects of "hero" object.

this is the declaration I made:

vector<hero*> arr;

and these are the lines entering the object into the array(archer is an object inhert from hero)

archer a('a'+to_string(numPlayer),gend,hit, point, point2); 

 this->arr.insert (this->arr.begin() + numPlayer, &a);

It seems like everything is working, but when I try to get the health of the object, it show me 6 but it should be 100. why is that? I also try to get the name of the object but it gives me segmentation fault. why does it happend?

this is hero.h:

 #ifndef HERO_H
#define HERO_H
#include <string>
#include <math.h>
#include <map>
#include <iostream>
#include "point2d.hpp"
//#include "enemy.hpp"
using namespace std;
#include "actors.hpp"

class hero:public actors{
    protected:
        string name; 
        int size; 
        char gender; 
        double health; 
        double hitting;
        point2d point, goal;
        double radius;


    public:

        hero(string name, char gender, double damage, point2d point, point2d goal);
        hero(const  hero &hero);
        ~hero();
        string getName();
        char getGender();
        double getDamage();
        point2d getPoint();
        double getHealth();

        void setName(string name);
        void setGender(char gender);
        void setDamage(double damage);
        void setHealth(double health);
        void setPoint(point2d point);
        string toString(); ///const in the end!!!!
        bool move(map<string, char> &matrix, vector<potion> potions );
        double getRadius();


       void hurt(double hit);

};

class warrior :public hero{
public:

    warrior(string name, char gender, double damage, point2d point, point2d point2);
    warrior(warrior &w);
    ~warrior();




};

class archer :public hero{
public:

    archer(string name, char gender,  double damage, point2d point, point2d point2);
    archer(archer &a);
    ~archer();



};

class wizard :public hero{
public:

    wizard(string name, char gender,  double damage, point2d point, point2d point2);
    wizard(wizard &a);
    ~wizard();
};

#endif

and these are archer and hero constructors

hero::hero(string name, char gender, double damage, point2d point,point2d point2){    
        this->name = name;
        this->gender=gender;
        this->health=100;
        this->hitting=damage;
        this->point = point2d(point);
        this->goal=point2d(point2);
        this->size=0;

}

archer::archer(string name, char gender, double damage, point2d point, point2d point2):
    hero(name, gender,  damage, point, point2) {

}

this is the get: cout<<this->arr.at(0)->getHealth(); // output is 6

cout<<this->arr.at(0)->getName(); 

//output:bash: line 12: 23084 Segmentation fault $file.o $args

adi
  • 21
  • 1
  • 1
    The right tool to solve such problems is your debugger. You should step through your code line-by-line *before* asking on Stack Overflow. For more help, please read [How to debug small programs (by Eric Lippert)](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). At a minimum, you should \[edit] your question to include a [Minimal, Complete, and Verifiable](http://stackoverflow.com/help/mcve) example that reproduces your problem, along with the observations you made in the debugger. – πάντα ῥεῖ Dec 07 '16 at 10:03
  • 1
    `&a` ? Something tells me by the time you actually perform your inspection and/or name print from the pointer you're storing in that vector that `a` has *long* since been destroyed due to exiting the scope where it was created. In short, my crystal ball tells me you're loading your vector with what ultimately becomes a dangling pointer. – WhozCraig Dec 07 '16 at 10:08
  • BTW, the warrior, archer and wizard classes do NOT have copy constructors (missing const keyword). – Dmitry T. Dec 07 '16 at 10:10
  • First of all do you really need to have vector of pointers? Why not just store it directly(vector arr)? Also your parent class doesn't have a virtual destructor which may cause undefined behaviour. – Maroš Beťko Dec 07 '16 at 10:17
  • @MarošBeťko In such situation, the author could not use the polymorphism due to object slicing – alexeykuzmin0 Dec 07 '16 at 10:20

1 Answers1

1

The problem is in the way of adding the heroes to vector:

archer a('a' + to_string(numPlayer), gend, hit, point, point2); 
this->arr.insert(this->arr.begin() + numPlayer, &a);

Here you insert a pointer to local variable to your storage. When the execution flow leaves the current block, your archer a is destroyed, and the memory it uses may be used for other objects (which change the former health field as well as pointer to the name). For more information, please check this.

To fix this, you should allocate your archer on heap, like this:

archer* a = new archer('a' + to_string(numPlayer), gend, hit, point, point2); 
this->arr.insert(this->arr.begin() + numPlayer, a);

This code will probably work correctly, but there's one more flaw: looks like the container arr should own all the heroes belonging to it, managing their life time to avoid memory (and other resources) leaks. To achieve this, you may use the smart pointers, like this:

vector<unique_ptr<hero>> arr;
...
auto a = make_unique<archer>('a' + to_string(numPlayer), gend, hit, point, point2); 
this->arr.insert(this->arr.begin() + numPlayer, move(a));

For more information on smart pointers, please see these:

  1. Cppreference documentation of make_unique.
  2. Core c++ guidelines, they have some rules about smart pointers as well as on many other themes.
LW001
  • 2,452
  • 6
  • 27
  • 36
alexeykuzmin0
  • 6,344
  • 2
  • 28
  • 51