Public Group

# Stat formatter - Check for elegence & efficiency

This topic is 4130 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

Hi, I've started making a text RPG, and I'm in the process of creating the battle system. I've written some code that displays each characters' stats, but I'd like some ideas on how to improve it... Here's the code (its in cpp):
//Module displayPlayerStats(playerlist, int count)
//Make sure that the alias of each character is not too long
//If it is, shorten it
//Determine the longest line
//Store the length of the longest line
//If the line is the longest, pad 2 spaces
//If the line is shorter than the longest
//Add enough padding to the end of the line to make it an equal length to the longest line
//Do the same for all three players, while concatonating the strings together
void displayPlayerStats(PLAYER* playerList, int count)
{
//Declare local variables
string alias, health, mindMagic, strHolder;
string aliasLine = "";
string healthLine = "";
string mindMagicLine = "";

for (int i = 0; i < count; i++)
{
//Set the alias string to the ith players name
alias = playerList.getAlias();

//If the alias + the padding is longer than the screen width/3
if (alias.length()+2 > 78/3)
{
//Erase everything after the screen width/3 - (padding + 3 dots)
alias.erase(78/3 - 5);

//Add three dots to the players name indicating that it has been shortened
alias.append(3, '.');
}

//Write the health line
health = "HP:";
stringstream sStreamHealth;
sStreamHealth << playerList.getHealth();

strHolder;

sStreamHealth >> strHolder;
health += strHolder;

health += "/";

stringstream sStreamMaxHealth;

sStreamMaxHealth << playerList.getMaxHealth();
sStreamMaxHealth >> strHolder;

health += strHolder;

//Write the magic line
mindMagic = "MP:";
stringstream sStreamMindMagic;
sStreamMindMagic << playerList.getMana();

sStreamMindMagic >> strHolder;
mindMagic += strHolder;

mindMagic += "/";

stringstream sStreamMaxMindMagic;

sStreamMaxMindMagic << playerList.getMaxMana();
sStreamMaxMindMagic >> strHolder;

mindMagic += strHolder;

//Get the length of the longest line
int longLine;
longLine = 0;

//Find out the longest string
if (alias.length() > longLine)
longLine = alias.length();
if (health.length() > longLine)
longLine = health.length();
if (mindMagic.length() > longLine)
longLine = mindMagic.length();

if (alias.length() == longLine)
{
alias += "  ";
}
else if (alias.length() < longLine)
{
alias.append(longLine - alias.length() + 2, ' ');
}

if (health.length() == longLine)
{
health += "  ";
}
else if (health.length() < longLine)
{
health.append(longLine - health.length() + 2, ' ');
}

if (mindMagic.length() == longLine)
{
mindMagic += "  ";
}
else if (mindMagic.length() < longLine)
{
mindMagic.append(longLine - mindMagic.length() + 2, ' ');
}

//Save the entire line
aliasLine += alias;
healthLine += health;
mindMagicLine += mindMagic;

}

//Display all stats
cout << aliasLine << "\n";
cout << healthLine << "\n";
cout << mindMagicLine << "\n\n";
}


Is there a better way to typecast the int to string? Is there a more elegant way to approach this problem? -John [Edited by - anothrguitarist on March 2, 2007 12:24:35 PM]

##### Share on other sites
Use std::vector<> instead of a raw array of players.
Replace "magic numbers" (78 for the screen width) with constants.
Get Boost, then use boost::lexical_cast<> for conversion between int <-> string (alternatively, reimplement lexical_cast<> yourself; it is essentially the same as what you're doing, just wrapped up nicely into a function).

Most of the rest is style, which I won't bother arguing. Your comments, however, are mostly vacuous.

##### Share on other sites
Quote:
 Original post by jpetrieUse std::vector<> instead of a raw array of players.Replace "magic numbers" (78 for the screen width) with constants.Get Boost, then use boost::lexical_cast<> for conversion between int <-> string (alternatively, reimplement lexical_cast<> yourself; it is essentially the same as what you're doing, just wrapped up nicely into a function).Most of the rest is style, which I won't bother arguing. Your comments, however, are mostly vacuous.

Thanks for the advice. I changed the 'magic numbers' to contants, and I used your method of string <-> int conversion. However, I do not understand how to use vectors, can you elaborate on the subject or point me to a good tutorial.
Can I make a 'vector of an object'?
Can I make a a 'pointer vector'?
(Please excuse me if those questions aren't worded properly, I don't know the terminology used to describe vectors)

I've posted the updated code, just in case I'm not implementing the aforementioned techniques correctly:

//Module displayPlayerStats(playerlist, int count)//Make sure that the alias of each character is not too long	//If it is, shorten it//Determine the longest line//Store the length of the longest line//If the line is the longest, pad 2 spaces//If the line is shorter than the longest	//Add enough padding to the end of the line to make it an equal length to the longest line//Do the same for all three players, while concatonating the strings togethervoid displayPlayerStats(PLAYER* playerList, int count){	//Declare local variables	string alias, health, mindMagic;	string aliasLine = "";	string healthLine = "";	string mindMagicLine = "";	const int SCREEN_WIDTH = 78;	const int SCREEN_DIVISION = 3;	const int PADDING = 2;	const int NUM_OF_DOTS = 3;	for (int i = 0; i < count; i++)	{		//Set the alias string to the ith players name		alias = playerList.getAlias();		//If the alias + the padding is longer than the screen width/3		if (alias.length()+2 > SCREEN_WIDTH/SCREEN_DIVISION)		{			//Erase everything after the screen width/3 - (padding + 3 dots)			alias.erase(SCREEN_WIDTH/SCREEN_DIVISION - (PADDING + NUM_OF_DOTS));			//Add three dots to the players name indicating that it has been shortened			alias.append(NUM_OF_DOTS, '.'); 		}		//Write the health line		health = "HP:";		health += boost::lexical_cast<std::string>(playerList.getHealth());		health += "/";		health += boost::lexical_cast<std::string>(playerList.getMaxHealth());		//Write the magic line		mindMagic = "MP:";		mindMagic += boost::lexical_cast<std::string>(playerList.getMana());		mindMagic += "/";		mindMagic += boost::lexical_cast<std::string>(playerList.getMaxMana());		//Get the length of the longest line		int longLine;		longLine = 0;				//Find out the longest string		if (alias.length() > longLine)			longLine = alias.length();		if (health.length() > longLine)			longLine = health.length();		if (mindMagic.length() > longLine)			longLine = mindMagic.length();		//Pad the alias		if (alias.length() == longLine)		{			alias.append(PADDING, ' ');		}		else if (alias.length() < longLine)		{			alias.append(longLine - alias.length() + PADDING, ' ');		}		//Pad the health		if (health.length() == longLine)		{			health.append(PADDING, ' ');		}		else if (health.length() < longLine)		{			health.append(longLine - health.length() + PADDING, ' ');		}		//Pad the mind magic		if (mindMagic.length() == longLine)		{			mindMagic.append(PADDING, ' ');		}		else if (mindMagic.length() < longLine)		{			mindMagic.append(longLine - mindMagic.length() + PADDING, ' ');		}			//Save the entire line			aliasLine += alias;			healthLine += health;			mindMagicLine += mindMagic;	}	//Display all stats	cout << aliasLine << "\n";	cout << healthLine << "\n";	cout << mindMagicLine << "\n\n";}

EDIT: Looks like I still need to write better comments, but I can figure that out :P.

##### Share on other sites
Quote:
 Original post by anothrguitaristThanks for the advice. I changed the 'magic numbers' to contants, and I used your method of string <-> int conversion. However, I do not understand how to use vectors, can you elaborate on the subject or point me to a good tutorial. Can I make a 'vector of an object'?Can I make a a 'pointer vector'?

#include <iostream>#include <vector>int main(){    std::vector<int> foo;  // Vector of ints    std::vector<int*> bar; // Vector of pointers-to-int    int x = 5;    foo.push_back(x);  // Insert a copy of x at the back of the vector    bar.push_back(&x); // Insert a pointer to x at the back of the vector    /* Iterate over every element in the array, dereferencing the iterator (it's       sort of like a pointer (it actually encapsulates one, IIRC)) to print                each value in the vector.       Note that a typedef is normally used - things like         std::vector<int>::iterator       are a mouthful (and can get much worse than that!) */    for (std::vector<int>::const_iterator i = foo.begin(); i != foo.end(); ++i)        std::cout << (*i) << "\n";    if(!bar.empty())    // Check that our vector of pointers isn't empty first        bar.pop_back(); // If not, remove the last element from it    int *y = new int; // Allocate    *y = 10;          // ... and assign to a new int on the heap    bar.push_back(y); /* A copy of the pointer (still pointing to the same                          int)is placed at the back of the vector */    /* Range-checked accessing element of vector (can use indexing operator but                 it doesn't check the range) */    std::cout << "0th element of bar " << bar.at(0);    for (int i = 0; i < 10; ++i)        foo.push_back(i);   // Insert values from 0 to 9 into the vector    for (std::vector<int>::iterator i = foo.begin(); i != foo.end(); ++i)        (*i) *= 2;          // Scale every value in the vector by 2    // Iterates from end to beginning: prints 18, 16, 14 - etc.    for (std::vector<int>::reverse_iterator i = foo.rbegin(); i != foo.rend(); ++i)        std::cout << (*i) << ", ";    for (std::vector<int*>::iterator i = bar.begin(); i != bar.end(); ++i)       delete *i; // Clean up memory allocations    foo.clear();    bar.clear(); // Erase all elements in both vectors}

##### Share on other sites
Quote:
Original post by TheUnbeliever
Quote:
 Original post by anothrguitaristThanks for the advice. I changed the 'magic numbers' to contants, and I used your method of string <-> int conversion. However, I do not understand how to use vectors, can you elaborate on the subject or point me to a good tutorial. Can I make a 'vector of an object'?Can I make a a 'pointer vector'?

*** Source Snippet Removed ***

Thanks that helped a lot :).

By the way, why are vectors better than raw arrays? Is it because of memory leaks?

I've updated the code to include vectors. Are there any further optimizations?

[source lang = "cpp"]void displayPlayerStats(std::vector<PLAYER*> playerList, int count){	//Declare local variables	string alias, health, mindMagic;	string aliasLine = "";	string healthLine = "";	string mindMagicLine = "";	const int SCREEN_WIDTH = 78;	const int SCREEN_DIVISION = 3;	const int PADDING = 2;	const int NUM_OF_DOTS = 3;	for (int i = 0; i < count; i++)	{		//Set the alias string to the ith players name		alias = playerList.at(i)->getAlias();		//If the alias + the padding is longer than the screen width/3		if (alias.length()+2 > SCREEN_WIDTH/SCREEN_DIVISION)		{			//Erase everything after the screen width/3 - (padding + 3 dots)			alias.erase(SCREEN_WIDTH/SCREEN_DIVISION - (PADDING + NUM_OF_DOTS));			//Add three dots to the players name indicating that it has been shortened			alias.append(NUM_OF_DOTS, '.'); 		}		//Write the health line		health = "HP:";		health += boost::lexical_cast<std::string>(playerList.at(i)->getHealth());		health += "/";		health += boost::lexical_cast<std::string>(playerList.at(i)->getMaxHealth());		//Write the magic line		mindMagic = "MP:";		mindMagic += boost::lexical_cast<std::string>(playerList.at(i)->getMana());		mindMagic += "/";		mindMagic += boost::lexical_cast<std::string>(playerList.at(i)->getMaxMana());		//Get the length of the longest line		int longLine;		longLine = 0;				//Find out the longest string		if (alias.length() > longLine)			longLine = alias.length();		if (health.length() > longLine)			longLine = health.length();		if (mindMagic.length() > longLine)			longLine = mindMagic.length();		//Pad the alias		if (alias.length() == longLine)		{			alias.append(PADDING, ' ');		}		else if (alias.length() < longLine)		{			alias.append(longLine - alias.length() + PADDING, ' ');		}		//Pad the health		if (health.length() == longLine)		{			health.append(PADDING, ' ');		}		else if (health.length() < longLine)		{			health.append(longLine - health.length() + PADDING, ' ');		}		//Pad the mind magic		if (mindMagic.length() == longLine)		{			mindMagic.append(PADDING, ' ');		}		else if (mindMagic.length() < longLine)		{			mindMagic.append(longLine - mindMagic.length() + PADDING, ' ');		}			//Save the entire line			aliasLine += alias;			healthLine += health;			mindMagicLine += mindMagic;	}	//Display all stats	cout << aliasLine << "\n";	cout << healthLine << "\n";	cout << mindMagicLine << "\n\n";}

[Edited by - anothrguitarist on March 2, 2007 7:32:06 PM]

1. 1
2. 2
3. 3
4. 4
Rutin
17
5. 5

• 11
• 32
• 12
• 12
• 11
• ### Forum Statistics

• Total Topics
631409
• Total Posts
2999929
×