Pointer pointing to wrong address?

Question

I have a problem with pointer(s) which seems to point to wrong address. I have a function inside of League class to create the schedule:

void League::CreateSchedule()
{
 for (int i = 0; i < allTeamsInLeague.size(); i++)
 {
    int tempDay = startingDay, tempMonth = startingMonth, tempYear =         Time::GetInstance().GetYear();
    for (int y = 0; y < allTeamsInLeague.size(); y++)
    {
        if (i != y)
        {
            allNotPlayedLeagueMatches.push_back(NotPlayedMatch(allTeamsInLeague[i], allTeamsInLeague[y], tempDay, tempMonth, tempYear));
            allTeamsInLeague[i]->GetMyMatches().push_back(&allNotPlayedLeagueMatches[allNotPlayedLeagueMatches.size() - 1]);
            allTeamsInLeague[y]->GetMyMatches().push_back(&allNotPlayedLeagueMatches[allNotPlayedLeagueMatches.size() - 1]);
            std::cout <<"Size of allNotPlayedLeagMatches: " << allNotPlayedLeagueMatches.size() << std::endl;
        }
    }
 }
std::cout <<"League shedule done!" <<std::endl;
}

It adds the address of the match created in the allNotPlayedLeagueMatches vector(of NotPlayedLeagueMatches) to the vector myMatches(vector(Match*) where Match is pure abstract class) of the class Team.

std::vector<Match*> &Team::GetMyMatches()
{
return myMatches;
}

Then when I try to output the teams' matches it crashes when displaying myMatches[0], but it's not crashing when displaying just myMatches[1]. I tried to use the debugger to solve this problem and it says unable to read memory for myMatches[0].

Here is the League.h file:

#pragma once
#include "PlayedMatch.h"
#include "Time.h"
#include <vector>
#include <iostream>

class League
{
public:

//Default Constructor
League();

//Overloaded Constructor
League(std::string name, int startingDay, int startingMonth);

//Destructor
~League();

std::vector<Team*> &GetAllTeamsInLeague();

void CreateSchedule();

private:

std::vector<Team*> allTeamsInLeague;

std::string name;

int startingDay, startingMonth;

std::vector<NotPlayedMatch> allNotPlayedLeagueMatches;
std::vector<PlayedMatch> allPlayedLeagueMatches;
};

League.cpp:

#include "League.h"


League::League()
{
name = "";
startingDay = 1;
startingMonth = 1;
}


League::~League()
{

}

League::League(std::string name, int startingDay, int startingMonth)
{
this->name = name;
this->startingDay = startingDay;
this->startingMonth = startingMonth;
}

std::vector<Team*> &League::GetAllTeamsInLeague()
{
return allTeamsInLeague;
}

void League::CreateSchedule()
{
for (int i = 0; i < allTeamsInLeague.size(); i++)
{
    int tempDay = startingDay, tempMonth = startingMonth, tempYear =Time::GetInstance().GetYear();
    for (int y = 0; y < allTeamsInLeague.size(); y++)
    {
        if (i != y)
        {
            allNotPlayedLeagueMatches.push_back(NotPlayedMatch(allTeamsInLeague[i], allTeamsInLeague[y], tempDay, tempMonth, tempYear));
            allTeamsInLeague[i]->GetMyMatches().push_back(&allNotPlayedLeagueMatches[allNotPlayedLeagueMatches.size() - 1]);
            allTeamsInLeague[y]->GetMyMatches().push_back(&allNotPlayedLeagueMatches[allNotPlayedLeagueMatches.size() - 1]);
            std::cout <<"Size of allNotPlayedLeagMatches: " << allNotPlayedLeagueMatches.size() << std::endl;
        }
    }
}
std::cout <<"League shedule done!" <<std::endl;

}

Team.h:

#ifndef Team_H
#define Team_H

#include <iostream>
#include <string>
#include <vector>
#include "Match.h"

using namespace std;

class Team
{

public:
//Default Constructor
Team();

//Overloaded Constructor string name, int defense, int attack
Team(string, int, int, int, int);

//Destructor
~Team();

void ShowTeamCharacteristics();
//shows the team characteristics(defence, attack, name...)

int GetAttack();
//Returns team's attack ability

int GetDefense();
//Returns team's defence ability

int GetLeagueX();
//sets in which country the team is

int GetLeagueY();
//sets in which league the team is

void ShowCalendar();


string GetName();
//Returns team's name

void SetAttack(int);
//sets the new value to the current attack 

void SetDefense(int);
//sets the new value to the current defense 

void SetName(string);
//sets the new value to the current name

void SetLeagueX(int);
//sets in which country the team is

void SetLeagueY(int);
//sets in which league the team is

std::vector<Match*> &GetMyMatches();

private:

int defense, attack; // 100 - defense = chance to concede a goal, attack / 1.5 = chance to score a goal
string name; // The name of the team
int leagueX, leagueY;
int points;
std::vector<Match*> myMatches;


};
#endif

Team.cpp:

#include "Team.h"

Team::Team()
{
name = "";
attack = 0;
defense = 0;
leagueX = 0;
leagueY = 0;
points = 0;
}

Team::Team(string name, int defense, int attack, int leagueX, int leagueY)
{
this->name = name;
this->defense = defense;
this->attack = attack;
this->leagueX = leagueX;
this->leagueY = leagueY;
points = 0;
}


Team::~Team()
{
}

void Team::ShowCalendar()
{
for (int i = 0; i < myMatches.size(); i++)
{
    myMatches[i]->ShowMatchDetails();
}
}

std::vector<Match*> &Team::GetMyMatches()
{
return myMatches;
}


void Team::ShowTeamCharacteristics()
{
system("cls");
cout <<"Name: " << name << endl <<
    "Attack: " << attack << endl <<
    "Defense: " << defense << endl;
}

int Team::GetAttack()
{
return attack;
}

int Team::GetDefense()
{
return defense;
}

int Team::GetLeagueX()
{
return leagueX;
}

int Team::GetLeagueY()
{
return leagueY;
}

string Team::GetName()
{
return name;
}

void Team::SetAttack(int attack)
{
this->attack = attack;
}

void Team::SetDefense(int defense)
{
this->defense = defense;
}

void Team::SetName(string name)
{
this->name = name;
}

void Team::SetLeagueX(int num)
{
leagueX = num;
}

void Team::SetLeagueY(int num)
{
leagueY = num;
}

Any idea what's wrong?

Answer 1

The problem is that the elements in your allNotPlayedLeagueMatches vector might get moved around as you add additional elements.

Imagine the following scenario: You push an element to a vector. A fixed amount of memory is allocated for the vector storage and the object is placed there. You now take a pointer to that location in memory and store it somewhere.

Now you push additional elements into the vector until it runs out of capacity. So the vector allocates a bigger portion of memory, copies all the elements to that new memory location and then deallocates the original storage.

Problem is that your pointer still points to the old location and is now dangling. Dereferencing the pointer might still work as long as no one touches that memory, but you still have a bug here (dereferencing a dangling pointer is undefined behavior).

One way to get around this is to only take pointers to vector elements after all elements have been inserted. Of course that means your vector has to be 'frozen' at a certain moment in your program and no more changes are allowed to it. If that is not an option, consider using a different container that does not move objects around upon insertion (like a std::list).