0

i have a class (Team) which contains 2 AVL trees - one AVL tree's nodes contain data of shared_ptr to an object of class Player, and the other AVL tree's nodes contain data of shared_ptr to an object of class PlayerKeyId - which is a successor of class Player and the only differents between those classes are the operators "<" and ">". the goal was to organize the players in the team by 2 categories. after using this structure for a lot of functions I found out I have a lot of memory leaks because I used the default constructor ,copy constructor and operator "=". i tried to write those for each class but I have some SEG FAULT problems.

   //start of relevant code - class Player
class Player{
    public:
    int playerID;
    int goals;
    int cards;
    int playerGamesPlayed;
    bool goalkeeper;
    Node<shared_ptr<Player>>* followingPlayer;
    Node<shared_ptr<Player>>* previousPlayer;
    shared_ptr<Team> playerTeamPointer;
    int teamPlayerId;
    public:
    
    Player(int playerID,int teamID, int gamesPlayed, int goals,int cards,bool goalkeeper,  shared_ptr<Team> playerTeamPointer):playerID(playerID),goals(goals),cards(cards),playerGamesPlayed(gamesPlayed),goalkeeper(goalkeeper),playerTeamPointer(playerTeamPointer),teamPlayerId(teamID){

    };
    
    ~Player(){
            playerTeamPointer=nullptr;
    };

    Player( Player & p){
        followingPlayer = new Node<shared_ptr<Player>>();
        previousPlayer = new Node<shared_ptr<Player>>();
        playerID = p.playerID;
        goals = p.goals;
        cards = p.cards;
        playerGamesPlayed = p.playerGamesPlayed;
        goalkeeper = p.goalkeeper;
        *(followingPlayer)= *(p.followingPlayer);
        *(previousPlayer) = *(p.previousPlayer);
        teamPlayerId = p.teamPlayerId;
    }

     Player& operator=(Player& p){
        playerID = p.playerID;
        goals = p.goals;
        cards = p.cards;
        playerGamesPlayed = p.playerGamesPlayed;
        goalkeeper = p.goalkeeper;
        *(followingPlayer)= *(p.followingPlayer);
        *(previousPlayer) = *(p.previousPlayer);
        teamPlayerId = p.teamPlayerId;
        return *this;
    }

    Player(){}

//end of relevant code - class Player

//start of code - class Team.

class Team{
    public:
    int teamID;
    int points;
    int playerCount;
    int teamGames;
    shared_ptr<Player> topScorer;
    int totalGoals;
    int totalCards;
    int goalKeepers;
    bool valid;
    AVLTree<shared_ptr<PlayerKeyId>> *playersId;
    AVLTree<shared_ptr<Player>> *playersGoals;

    Team(int teamID, int points):teamID(teamID),points(points){
        playerCount=0;
        teamGames=0;
        topScorer = make_shared<PlayerKeyId>(-1,-1,-1,-1,-1,false,nullptr);
        totalGoals=0;
        totalCards=0;
        goalKeepers=0;
        valid=false;

        playersId = new  AVLTree<shared_ptr<PlayerKeyId>>();
        playersGoals = new  AVLTree<shared_ptr<Player>>();
    }

    Team( Team & t){
        playersId = new AVLTree<shared_ptr<PlayerKeyId>>();
        playersGoals = new AVLTree<shared_ptr<Player>>();
        teamID = t.teamID;
        points = t.points;
        playerCount = t.playerCount;
        teamGames = t. teamGames;
        topScorer = t.topScorer;
        totalCards = t.totalCards;
        goalKeepers = t.goalKeepers;
        valid = t.valid;
        *(playersId) = *(t.playersId);
        *(playersGoals) = *(t.playersGoals);
    }

    Team& operator =(Team& t){
       teamID = t.teamID;
        points = t.points;
        playerCount = t.playerCount;
         teamGames = t. teamGames;
        topScorer = t.topScorer;
        totalCards = t.totalCards;
        goalKeepers = t.goalKeepers;
        valid = t.valid;
        *( playersId) = *(t. playersId);
        *( playersGoals) = *(t. playersGoals);
       return *this;
    }

    ~Team(){
        topScorer=nullptr;
        delete playersId;
        delete playersGoals;
    }

//end of code relevant - class Team.

//**in class PlayerKeyId I didn't wrote those three because I wanted to use the ones of Player. here is the start of PlayerKeyId:

class PlayerKeyId:public Player{
    public:

     using Player::Player;

//start of relevant code - AVL tree

template <class T>
class Node {
public:
    Node* left;
    T data;
    Node* right;
    int height;
     Node* parent;

     Node(){
        left=NULL;
        right=NULL;
        height=0;
        parent=NULL;
     }
  
     ~Node(){
        parent = nullptr;
     }

     Node<T>(Node<T>& oldNode){
        if(oldNode.left!=NULL)
            left= new Node<T>;
         if(oldNode.right!=NULL)
            right= new Node<T>;
         if(oldNode.parent!=NULL)
            parent= new Node<T>;
        data = new T();
        *left = oldNode.left;
        *right = oldNode.right;
        *parent = oldNode.parent;
        height=oldNode.height;
        *data = oldNode.data;
     }

     Node<T>& operator=(Node<T>& oldNode){
        data=oldNode.data;
       
            left=oldNode.left;
        
          right=oldNode.right;
        return *this;
     }

};
    
template <class T>
class AVLTree{
public:
    Node<T>* root;
   
    AVLTree(){ root = NULL; }

    AVLTree<T>& operator=(AVLTree<T>& oldAVL){
        root = oldAVL.root;
        return *this;
    }

     AVLTree(AVLTree& oldAVL)
     {
         root = new Node<T>;
        *(root) = oldAVL.root;
     }

//end of relevant code - AVL tree

I tried to look how to write those 3 in the right way but something went wrong on the road.

Algo
  • 185
  • 6
  • 1
    Custom copy/move constructors, assignments, and destructors are advanced features. The tree class must have them, but the problem domain classes (`Player`, `Team`, etc) should not. Same for manual `new`/`delete` calls, they should be limited to the tree class. – HolyBlackCat Dec 07 '22 at 19:15
  • Is there any reason you make a custom tree class at all, why not use `std::map`? – HolyBlackCat Dec 07 '22 at 19:17
  • @HolyBlackCat I am not allowed to use map – Algo Dec 07 '22 at 19:21
  • Once you have the copy and move constructors working, take a look at [What is the copy-and-swap idiom?](https://stackoverflow.com/q/3279543/4581301) for a nearly fool proof assignment operator. It's not optimally performant, but I always use it as my starting point and often it's fast enough to also be the stopping point. – user4581301 Dec 07 '22 at 20:00
  • note: You may not be able to turn in an assignment with `map`, but it's good to start development with a known-good canned solution so you can get up and testing the actual business logic of an assignment quickly. Once you know the important stuff works, you can focus on replacing the scaffolding to meet the assignment requirements. – user4581301 Dec 07 '22 at 20:04

0 Answers0