Pointer to LinkedList of Pointer

[ehsantl]

Hi guys

I'm trying to finish my assignment, I stuck in a weird situation. I'm trying to create a pointer a linkedlist. This linkedlist contains pointer to a user-defined objects (City) and I'm trying to add City to this linkedlist and then iterate through the linked list to find and modify them.

Here is my City.h source code:

#include "List.h"
#include <iostream>
#include <string>


class City {
private:
    std::string name;
    List<City*> *nextCities;
    bool visited;

public:
    City(std::string);
    void setNextCity(City c);
    bool isVisited();
    void setVisited();
    std::string getName();
    City* getNextUnvisitedCity();
};


City::City(std::string _name) {
    name = _name;
    visited = false;
    nextCities = new List<City*>();
    nextCities->push_back(this);
}

std::string City::getName() {
    return this->name;
}

void City::setNextCity(City city) {
    nextCities->push_back(&city);
};

bool City::isVisited() {
    return visited;
}

void City::setVisited() {
    visited = true;
}

City* City::getNextUnvisitedCity() {

    List<City*>::iterator start = (*nextCities).begin();
    List<City*>::iterator end = (*nextCities).end();

    while(start != end) {
        std::string ee = (*start)->getName();
        *start++;
    }

    //temp
    return NULL;
}

everything seems right when I try to add element into linkedlist:

void City::setNextCity(City city) {
    nextCities->push_back(&city);
};

But when I try to iterate through them. I get memory violation error. I'm not sure that I defined the pointer correctly or not.

I'm using this version of linkedlist implementation with iterator:
http://fscked.org/writings/225notes/week4/c++std/list225.h

[Ancient Dragon]

void City::setNextCity(City city) {
    nextCities->push_back(&city);
};

You can't do it like that because the parameter is a copy of City class, and will be destroyed as soon as setNextCity() exits. You need to make list own all the memory so that it doesn't rely on anything outside of it.

Just off the top of my head this is how you will have to do it. Note that you will also have to add a copy constructor to City class.

void City::setNextCity(const City& cty) {
    nextCities->push_back(new City(cty) );
};

[ehsantl]

void City::setNextCity(City city) {
    nextCities->push_back(&city);
};

You can't do it like that because the parameter is a copy of City class, and will be destroyed as soon as setNextCity() exits. You need to make list own all the memory so that it doesn't rely on anything outside of it.

Just off the top of my head this is how you will have to do it. Note that you will also have to add a copy constructor to City class.

void City::setNextCity(const City& cty) {
    nextCities->push_back(new City(cty) );
};

Thanks mate, I've almost forgot copy constructor in C++..It works fine :)