Создание списка объектов указателей

Question

Я пытаюсь создать список (который является частным memeber объекта) объектных указателей ...

Так что программа я кодирование является моделированием между двумя группами " Hero "s. Каждый герой имеет особые полномочия, физические атрибуты и т. Д. ... и список целей. Список целей - это список указателей на героя. Когда я вызываю конструктор для создания Гера, вся информация инициализируется со случайными значениями, за исключением списка целей. До сих пор я создал два списка героев, команду 1 и команду 2. Я пытаюсь создать список указателей в команде 1, который указывает на адрес Heros в команде 2 и наоборот. Серия for loops в main() - лучшее решение, которое у меня есть, но функция Hero :: setTarget «pushes_back» - адрес первого героя несколько раз. Любые советы будут очень признательны.

Вот что я до сих пор за исключением файлов Powers.h и Powers.cpp ...

Редакция: удалено вложенный для петель, но я все еще получаю тот же адрес, переданный в мой список ....

#include <iostream> 
    #include <list> 
    #include <random> 
    #include <ctime> 

    #include "Hero1.h" 
    #include "Powers.h" 

    using namespace std; 

    int main() 
    { 
///random number generator 
    default_random_engine generator(time(NULL)); 
    uniform_int_distribution<int> numHero(1,10); 

///lists of hero's, pointers to a hero, list iterators 
    list<Hero> team1; 
    list<Hero> team2; 
    Hero * hptr1; 
    Hero * hptr2; 
    list<Hero>::iterator hitr1; 
    list<Hero>::iterator hitr2; 

/// team 1 is created 
    int a = numHero(generator); 
    for(int x=0; x<a; x++) 
    { 
    hptr1 = new Hero(); 
    team1.push_back(*hptr1); 
    } 

/// team 2 is created 
    int b = numHero(generator); 
    for(int x=0;x<b;x++) 
    { 
    hptr2 = new Hero(); 
    team2.push_back(*hptr2); 
    } 

for(hitr2=team2.begin();hitr2!=team2.end();hitr2++) 
{ 
     hptr1->setTarget(hptr2); 
} 

for(hitr1=team1.begin();hitr1!=team1.end();hitr1++) 
{ 
    hptr2->setTarget(hptr1); 
} 

///printing results for list of targets 
int w =1; 
cout<<"target address "<<w<<endl; 
     hitr1=team1.begin(); 
     hptr1->displayTarget(); 

Герой мой Hero.h

#ifndef HERO1_H_INCLUDED 
#define HERO1_H_INCLUDED 

#include <iostream> 
#include <random> 
#include <ctime> 
#include <list> 

#include "Powers.h" 


using namespace std; 

class Power; 

class Hero 
{ 
public: 

    Hero(); 
    // ~Hero(); 

    void setID(); 
    void setLoc(); 
    void setPhy(); 
    void setPowers(); 
    void setEquip(); 
    void setTarget(Hero *h); 

    int getID(){return id;} 
    int getLoc(int x); 
    int getPhy(int x); 

    void displayHero(); 
    void displayTarget(); 
    void displayPowers(); 


private: 
    int id; 
    int location[3]; 
    int physical [4]; 
    Power * powPtr; 
    Power * equipPtr; 
    Hero * targetPtr; 
    list<Power> powers; 
    list<Power> equipment; 
    list<Hero*> target; 
    list<Power>::iterator equipItr; 
    list<Power>::iterator powItr; 
    list<Hero*>::iterator targetItr; 

}; 

Мои Hero.cpp

#include "Hero1.h" 

default_random_engine generator(time(NULL)); 
uniform_int_distribution<int> distribution(100000,200000); 
normal_distribution<double> disto(50,10); 
uniform_int_distribution<int> randPow(1,3); 

Hero::Hero() 
{ 
    setLoc(); 
    setID(); 
    setPhy(); 
    setPowers(); 
    setEquip(); 
} 

void Hero::setLoc() 
{ 
    location[0] = 1; 
    location[1] = 2; 
    location[2] = 3; 
} 

void Hero::setID() 
{ 
    int a = distribution(generator); 
    id = a; 
} 

void Hero::setPhy() 
{ 
    double a = disto(generator); 
    double b = disto(generator); 
    double c = disto(generator); 
    double d = disto(generator); 

    physical[0] = a; 
    physical[1] = b; 
    physical[2] = c; 
    physical[3] = d; 

} 

void Hero::setPowers() 
{ 
    int a = randPow(generator); 
    for(int x=0;x<a;x++) 
    { 
    powPtr = new Power(getPhy(3)); 
    powers.push_back(*powPtr); 


    } 
} 


void Hero::setEquip() 
{ 
    int a = randPow(generator); 
    for(int x=0;x<a;x++) 
    { 
    equipPtr = new Power(getPhy(3)); 
    powers.push_back(*equipPtr); 
    } 

} 
void Hero::setTarget(Hero *h) 
{ 

    target.push_back(h); 

} 

void Hero::displayTarget() 
{ 
    int x =1; 
    for(targetItr=target.begin();targetItr!=target.end();targetItr++) 
    { 
     cout<<&targetPtr<<endl; 
     x++; 
    } 
    cout<<x<<endl; 

} 

int Hero::getLoc(int x) 
{ 
    int p; 
    p = location[x]; 
    return p; 
} 

int Hero::getPhy(int x) 
{ 
    int p; 
    p = physical[x]; 
    return p; 
} 
void Hero::displayPowers() 
{ 
    cout<<"Number of powers = "<<powers.size()<<endl<<endl; 
    for(powItr=powers.begin();powItr!=powers.end();powItr++) 
    { 
     powPtr->displayEffect(); 
    } 
} 

void Hero::displayHero() 
{ 
    cout<<"Id :\t\t\t\t\t"<<id 
     <<"\nLocation:\t\t\t\t"<<location[0]<<","<<location[1]<<","<<location[2] 
     <<"\nPhysical attributes:\tstrength\t"<<physical[0]<<"\n\t\t\tendurance\t"<<physical[1] 
     <<"\n\t\t\tagility\t\t"<<physical[2]<<"\n\t\t\tspeed\t\t"<<physical[3]<<endl<<endl; 

     displayPowers(); 
} 

Answer 1

Я согласен с замечаниями @TheUndeadFish.

Далее, добавить следующие замечания:

list<Hero> team1; 
list<Hero> team2; 

Вместо того чтобы держать список объектов «Hero», было бы предложено сохранить список указателей на «Hero» объекты

list<Hero*> team1; 
list<Hero*> team2; 

Вышеуказанный предложенный метод полезен, так как he1-команда has1 содержит список героев team2 и наоборот, так что это поможет в доступе к обновленной информации каждого героя в качестве справочника указателей/ссылок в доступе к динамическим изменениям в объектных данных.

Кроме того, для циклов for нет инициализации hptr1 и hptr2 в исходном коде. Это может быть сделано следующим образом:

///lists of hero's, pointers to a hero, list iterators 
    list<Hero*> team1;     <-- change done here 
    list<Hero*> team2;     <-- change done here 
    Hero * hptr1; 
    Hero * hptr2; 
    list<Hero*>::iterator hitr1;   <-- change done here 
    list<Hero*>::iterator hitr2;   <-- change done here 

/// team 1 is created 
    int a = numHero(generator); 
    for(int x=0; x<a; x++) 
    { 
    hptr1 = new Hero(); 
    team1.push_back(hptr1);    <-- change done here 
    } 

/// team 2 is created 
    int b = numHero(generator); 
    for(int x=0;x<b;x++) 
    { 
    hptr2 = new Hero(); 
    team2.push_back(hptr2);    <-- change done here 
    } 

for(hitr2=team2.begin();hitr2!=team2.end();hitr2++) 
{ 
    hptr2 = *hitr2;      <-- change done here 
    for(hitr1=team1.begin();hitr1!=team1.end();hitr1++) 
    { 
     hptr1 = *hitr1;     <-- change done here 
     hptr1->setTarget(hptr2); 
    } 
} 

for(hitr1=team1.begin();hitr1!=team1.end();hitr1++) 
{ 
    hptr1 = *hitr1;      <-- change done here 
    for(hitr2=team2.begin();hitr2!=team2.end();hitr2++) 
    { 
    hptr2 = *hitr2;      <-- change done here 
    hptr2->setTarget(hptr1); 
    } 
} 

Answer 2

Я думаю, что я должен сказать, что это более достойный комментарий, а затем ответ на этот вопрос, однако причудливые правила StackOverflow не позволят мне оставлять комментарии. Тем не менее, я думаю, вам следует переименовать «setTarget» в «addTarget», поскольку «setTarget» означает, что для переменной есть одно значение, которое будет перезаписано каждый раз, когда вы его вызываете, тогда как «addTarget» лучше сообщает, что вы будете добавление цели в список.

Кроме того, вы должны использовать std::for_each с лямбда-функцией, а не с явными петлями. Я сомневаюсь, что это сделало бы заметную разницу в показателях производительности или правильности, но это сделало бы вашу программу более читаемой и узнав о функциях STL, поможет вам улучшить свой навык программирования.

Answer 3

Я вижу несколько различных проблемы в коде, но позволяет начать с одного вы спросили о ...

Ваших setTarget звонков делает то же самое каждый раз, потому что они называют с указателями, которые Арен вообще не обновляться внутри петель. Это проблема, которая может возникнуть в результате объявления всех ваших переменных в верхней части функции. Переменные, которые переживают свою цель, уязвимы для таких ошибок (или повторно используются в нескольких целях, а затем делают код более сложным для понимания и поддержки). Поэтому я настоятельно рекомендую определять локальные переменные точно там, где они нужны.

Например:

/// lists of heroes 
    list<Hero> team1; 
    list<Hero> team2; 

/// team 1 is created 
    int a = numHero(generator); 
    for(int x=0; x<a; x++) 
    { 
     Hero * hptr1 = new Hero(); 
     team1.push_back(*hptr1); 
    } 

/// team 2 is created 
    int b = numHero(generator); 
    for(int x=0;x<b;x++) 
    { 
     Hero * hptr2 = new Hero(); 
     team2.push_back(*hptr2); 
    } 

for(list<Hero>::iterator hitr2=team2.begin();hitr2!=team2.end();hitr2++) 
{ 

    for(list<Hero>::iterator hitr1=team1.begin();hitr1!=team1.end();hitr1++) 
    { 
     hptr1->setTarget(hptr2); 
    } 
} 

for(list<Hero>::iterator hitr1=team1.begin();hitr1!=team1.end();hitr1++) 
{ 
    for(list<Hero>::iterator hitr2=team2.begin();hitr2!=team2.end();hitr2++) 
    { 
     hptr2->setTarget(hptr1); 
    } 
} 

Если вы сделали это, то вы могли бы получить ошибки компиляции, потому что hptr1 и hptr2 не существует там, где setTarget вызывается. И это именно то, что вы хотите - компилятор обратить ваше внимание на ошибку.

Итак эти петли могут быть исправлены, как что-то вроде:

for(list<Hero>::iterator hitr2=team2.begin();hitr2!=team2.end();hitr2++) 
{ 

    for(list<Hero>::iterator hitr1=team1.begin();hitr1!=team1.end();hitr1++) 
    { 
     Hero& target = *hitr2; 
     hitr1->setTarget(&target); 
    } 
} 

Далее, для второй задачи: вы утечка памяти при заполнении списка:

Hero * hptr1 = new Hero(); 
team1.push_back(*hptr1); 

Это будет динамически выделять a Hero, но затем скопируйте значения этого в другой Герой, который создается в списке, потому что вот как push_back работает с чем-то вроде list<Hero>. И тогда указатель на динамически выделенный один выброшен (или повторно используется в коде, который вы изначально разместили), поэтому ничто не может освободить эту память.

То, что вы на самом деле хотите сделать, это больше похоже на:

team1.push_back(Hero()); 

Answer 4

Следующие разделы кода из исходной программы также требуют модификации:

void Hero::displayTarget() 
{ 
    int x = 0;         <-- change done here (not important) 
    for (targetItr=target.begin(); targetItr!=target.end(); targetItr++) 
    { 
     cout << *targetItr << endl;    <-- change done here 
     x++; 
    } 
    cout<<x<<endl; 
} 

Примечание:

1. член targetPtr в классе Hero не инициализируется в вашем оригинальный код.
2. В displayTarget() всегда targetPtr был напечатан, но не обновлен с каждой итерацией targetItr.

Кроме того, изменение в коде отображения в главном():

cout<<"target addresses in team 1"<<endl; 
     hitr1=team1.begin(); 
     hptr1 = *hitr1;      <-- change done here 
     cout<<"address of team 1 pointer "<<hptr1<<endl; 
     hptr1->displayTarget(); 
     cout<<endl<<"-------------"<<endl; 
     cout<<"target addresses in team 2"<<endl; 
     hitr2=team2.begin(); 
     hptr2 = *hitr2;      <-- change done here 
     cout<<"addreass of team 2 pointer "<<hptr2<<endl; 
     hptr2->displayTarget(); 

Надеется, что это помогает. Всего наилучшего.