Review of first program

[Yeti86]

Hello everybody,

I start c++ coding with a program for scouting a sportsgame.
Therefore I developed a class Player with different methods. Now I want to create a Team class which creates dynamically a number of Players in this "top" class.

Am I on the right way with this prototype of class team? I would be glad for any suggestions what I could do better, especially the constructor and the destructor of class team. I think the destructor doesn't work properly.

Further any suggestions of Variable names, class names or everything else is welcome.

Thanks in advance!

#include <iostream>
#include <string>
#include <cmath>
#include <fstream>
#include <vector>
namespace playerspace{
class player{
private:
	int number;
	int points;
	std::string name;
public:
	player(std::string, int, int);
	player(std::string, int );
	player();
//	~player();
	void set_name(std::string);
	void set_number(int);
	void set_points(int);
	void set_score_points_one();
	void set_score_points_two();
	void set_score_points_three();
	std::string get_name() const;
	int get_points() const;
	int get_number() const;

};
// method implementations....
}
namespace teamspace{
class team{
//private:
public:
	std::vector <playerspace::player*> Playervector;
	team(int number_of_players);
	~team();
	void set_up_numberofplayers(int number_of_players);
	void set_up_playername();
//	void set_up_team(int number_of_players);
};
void team::set_up_numberofplayers(int number_of_players)
{
	 team::Playervector.reserve(number_of_players);
};
void team::set_up_playername()
{	 std::string temp_playername;
	 for(unsigned int i=0; i<Playervector.capacity();i++)
	 {	
 		std::cout<<"Playername:"<<"\n";
		std::getline(std::cin,temp_playername);
		Playervector[i]->set_name(temp_playername);
	};
};
team::team(int number_of_players)
	{
		set_up_numberofplayers(number_of_players);
		for(int i=0;i<number_of_players;i++)
		{	//std::cout<<"allocator"<<Playervector.capacity()<<"\n";
			//std::cout<<Playervector.size()<<"\n";
			Playervector[i]=new playerspace::player;
		};
	};
team::~team()
	{

		for(int i=0;i<Playervector.capacity();i++)
			{

				delete Playervector[i];
			};
	};
}

[iAk3T05o]

Don't make game with the console. They are not really the same with games made with a library or game engine

[JLBorges]

Prefer value semantics; rather than a vector of pointers to objects, keep a vector of objects.

// std::vector <playerspace::player*> Playervector;
std::vector <playerspace::player> Playervector;

Once we do that, we should also remove the user defined destructor and let it be implicit.

The vector<> member function reserve() does not change the size of the vector; and capacity() does not tell us how many elements are there in the vector.

To change the size of a vector, use resize() http://www.cplusplus.com/reference/vector/vector/resize/

To determine the number of elements in the vector, use size() http://www.cplusplus.com/reference/vector/vector/size/

In this case, rather than resize(), we should probably add players to the vector with push_back(). http://www.cplusplus.com/reference/vector/vector/push_back/

See: http://www.mochima.com/tutorials/vectors.html

[Yeti86]

Thank you very much for the suggestions und detailled answer!

I thougt reserve() would be a good idea to initialize the vector. Is there no advantage because of the allocated space is all in a row?

[JLBorges]

> I thougt reserve() would be a good idea to initialize the vector.
> Is there no advantage because of the allocated space is all in a row?

If we have an estimate on the upper bound on the size to which a vector could grow, we could reserve space for that many elements, and there could be a performance gain.

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43

#include <iostream> #include <iomanip> #include <vector> void grow( std::vector<int>& vec, std::size_t n ) { std::cout << "initial size: " << std::setw(2) << vec.size() << " capacity: " << std::setw(2) << vec.capacity() << '\n' ; auto buffer = vec.data() ; for( std::size_t i = 0 ; i < n ; ++i ) { vec.push_back(i) ; if( buffer != vec.data() ) { std::cout << "relocated to a larger buffer. size: " << std::setw(2) << vec.size() << " capacity: " << std::setw(2) << vec.capacity() << '\n' ; buffer = vec.data() ; } } std::cout << " final size: " << std::setw(2) << vec.size() << " capacity: " << std::setw(2) << vec.capacity() << '\n' ; } int main() { std::cout << "\n-------- with reserve --------\n" ; { std::vector<int> vec ; vec.reserve(100) ; // reserve space for up to 100 elements grow( vec, 100 ) ; } std::cout << "\n-------- without reserve --------\n" ; { std::vector<int> vec ; grow( vec, 100 ) ; } }

-------- with reserve -------- initial size: 0 capacity: 100 final size: 100 capacity: 100 -------- without reserve -------- initial size: 0 capacity: 0 relocated to a larger buffer. size: 1 capacity: 1 relocated to a larger buffer. size: 2 capacity: 2 relocated to a larger buffer. size: 3 capacity: 4 relocated to a larger buffer. size: 5 capacity: 8 relocated to a larger buffer. size: 9 capacity: 16 relocated to a larger buffer. size: 17 capacity: 32 relocated to a larger buffer. size: 33 capacity: 64 relocated to a larger buffer. size: 65 capacity: 128 final size: 100 capacity: 128

http://coliru.stacked-crooked.com/a/ff0e271f3f3eac77

[Catfish666]

Using std::vector::reserve() is a good idea in general (the examples in the link below and also above by JLBorges show why), just don't confuse "reserve" with "resize".

http://www.cplusplus.com/reference/vector/vector/reserve/

Using reserve() is "cheaper" than resize() because memory allocation happens but objects are not constructed to fill in the vector.

Notice the difference in output in this example:
http://coliru.stacked-crooked.com/a/03704f01a4199cba

[Yeti86]

Thanks again very much!