Несколько вещей.
Во-первых, насколько я могу судить, NNodes просто отслеживает размер. Но у вас есть std::vector::size() для этого. Затем вы используете его, чтобы получить последний вставленный элемент, но вы можете просто использовать std::vector::back() для этого: return &N.back();.
Также ваш параметр передается по значению, когда его, вероятно, следует передавать по const-reference: const string& h. Это позволяет избежать ненужных копий, и в общем случае * вы должны передавать вещи по const-reference, а не по значению.
А это плохо:
node n;
N.push_back(n);
N[NNodes].setname(h);
node, вероятно, должен иметь конструктор, который принимает const string& и задает имя во время инициализации. Таким образом, у вас никогда не будет узла без имени, как в:
node n(h);
N.push_back(n);
Или более кратко:
N.push_back(node(h));
Намного лучше.
Во-вторых, да, vector может сделать недействительными указатели на элементы; а именно всякий раз, когда необходимо увеличить емкость вектора. Если вы можете, reserve() емкость заранее, чтобы избежать перераспределения. В вашем случае вы не можете, поэтому вы можете пойти двумя разными путями.
Первый маршрут - уровень косвенности . Вместо того, чтобы указывать прямо на вещи, поместите их индекс в массив. Обратите внимание, что хотя их адрес может измениться, их расположение в векторе не изменится. Вы бы Simulator::FindNode вернули size_t и вернули N.size() - 1. Добавьте члена типа node& GetNode(size_t index), который просто делает return N[index]; (будет проверка ошибок, если вы хотите). Теперь, когда вам нужен член, передайте индекс этому члену на GetNode, и вы получите ссылку на этот узел обратно.
Другой путь - поменять контейнер. Вы можете использовать deque, например. Это не имеет непрерывного хранения, но это очень похоже на vector. push_back и pop_back по-прежнему O (1), и он все еще имеет хорошую когерентность. (И, между прочим, deque обменивает непрерывное хранилище на способность к push_front и pop_front за O (1) раз)
Важно то, что deque будет не лишать законной силы указатели во время операции push или pop с любого конца. Он работает по типу гибридного векторного списка, где вы получаете куски памяти для элементов, связанных вместе. Измените базовое хранилище на deque (и не берите и не ставьте что-либо посередине), и вы можете просто указать на вещи.
Однако, насколько я могу судить, у вас ужасно неэффективная карта. Вы отображаете имена на узлы. Вы, вероятно, должны просто использовать std::map, который имеет точный интерфейс, который вы пытаетесь воссоздать. Вы даже можете указать на любой элемент на карте, который никогда не отменяет действия.
* Правило: передавайте по const-reference, если только тип не является примитивным (встроенным, как int, double и т. Д.), Если размер шрифта меньше sizeof(void*) или если вам потребуется его копия в любом случае.
То есть не делайте этого:
void foo(const std::string& s)
{
std::string ss(s); // make a copy, use copy
}
Но сделайте это:
void foo(std::string s) // make a copy, use copy
{
}