Need help with Dynamically allocating strings into arrays

[tay1392]

Hello everyone.

I have an error I've been trying to solve for a while that I can't seem to find a solution to.

Basically what I'm trying to do is create a Player object and store it into a dynamic array of players. I think that part works fine, but in my constructor where I allocate room for the players name, it crashes when I try to copy that into the player data member.. I can't seem to figure out why.

any help would be appreciated!

here's my code:
player.h

#include <iostream>
#ifndef PLAYER_H
#define PLAYER_H

class Player
{
	public:
		Player(const char * name, int grade, double gpa); //overloaded 
		~Player();
		bool Search();
		void Display();

	private:
		char * m_Name;
		int m_Grade;
		double m_Gpa;
};

#endif 

sport.h

#include <iostream>
#include "player.h"
#ifndef SPORT_H
#define SPORT_H

class Sport
{
	public:
		Sport();
		~ Sport();
		void Menu();
		void Add(Player **& PlayArray, int & count);
		void List();
		void Search() const;
		void DisplayCount();
		void SetCount(int count);
		void SetArray(Player ** ra);
		
	private:
		void PromptUser();
		Player ** m_PlayArray;
		int m_Length;
		int m_Count;

};

#endif 

the member function where I create a new player:

void Sport::Add(Player **& PlayArray, int & count)
{
	char name_buff[136];
	int grade_buff;
	double gpa_buff;

	Player ** temp = new Player * [m_Count + 1];
	
	for	(int i = 0; i < count; i++)
	{
		temp[i] = m_PlayArray[i];
	}
		
	cout << "Enter players name: ";
	cin >> name_buff;          // I know this won't work with spaces, its ok
	cout << "\nEnter players grade: ";
	cin >> grade_buff;
	cout << "\nEnter players GPA: ";
	cin >> gpa_buff;

	temp[m_Count] = new Player(name_buff, grade_buff, gpa_buff);
        //something wrong here ^^??
		
	delete [] m_PlayArray;
	m_PlayArray = temp;
	m_Count++;

}

and the overloaded constructor for my player objects:

Player::Player(const char * name, int grade, double gpa)
{

	name = new char [strlen(name) + 1];
	strcpy(m_Name, name);   // this is where it breaks!!

	m_Grade = grade;
	m_Gpa = gpa;
	cout << "Called" << endl;
}

Anyways, I could really use some help. Thank you!

[cire]

name = new char [strlen(name) + 1];
strcpy(m_Name, name); // this is where it breaks!

Well, that's because you didn't make your data member m_Name point to valid memory. name is not m_Name.

[Zaita]

I wouldn't suggest using strcpy anyways because it's vulnerable to buffer overflows.

I recommend you use std::string instead of char*s

[tay1392]

Cire, How would I fix that?

[cire]

Cire, How would I fix that?

The obvious way is to change name = new char[strlen(name)+1]; to m_Name = new char[strlen(name)+1]. You should consider using the constructor initialization list to initialize data members where possible.

Player::Player(const char * name, int grade, double gpa)
    : m_Name(new char[strlen(name)+1]), m_Grade(grade), m_Gpa(gpa)
{
    strcpy(m_Name, name);   // this is where it breaks!!
}

And, of course, Zaita is right. Using std::string here would be preferred if you're allowed to use it.

You're going to need a copy constructor and copy assignment operator as well. (If you're using C++11, a move constructor and move assignment operator too.) None of those would be necessary if you were using std::string.

[tay1392]

alright thanks Cire, I'll try that way.

Of course I would be using the string class but my teacher wants us to use pointers and dynamic memory allocation...unfortunately!

[Zaita]

He may want you to use pointers and dynamic allocation, but I would be highly surprised if he meant on everything.

Using both of those to hold instances of the Player class makes senses, swapping from a std::string to a char* does not.