Sign in to follow this  
EnigmaticCoder

Stat formatter - Check for elegence & efficiency

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[i].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[i].getHealth();

		strHolder;

		sStreamHealth >> strHolder;
		health += strHolder;

		health += "/";

		stringstream sStreamMaxHealth;

		sStreamMaxHealth << playerList[i].getMaxHealth();
		sStreamMaxHealth >> strHolder;

		health += strHolder;

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

		sStreamMindMagic >> strHolder;
		mindMagic += strHolder;

		mindMagic += "/";

		stringstream sStreamMaxMindMagic;

		sStreamMaxMindMagic << playerList[i].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();

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

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

		//Pad the mind magic
		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 this post


Link to post
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 this post


Link to post
Share on other sites
Quote:
Original post by jpetrie
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.


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 together
void 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[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[i].getHealth());
health += "/";
health += boost::lexical_cast<std::string>(playerList[i].getMaxHealth());

//Write the magic line
mindMagic = "MP:";
mindMagic += boost::lexical_cast<std::string>(playerList[i].getMana());
mindMagic += "/";
mindMagic += boost::lexical_cast<std::string>(playerList[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";
}




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

Share this post


Link to post
Share on other sites
Quote:
Original post by anothrguitarist
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'?



#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 this post


Link to post
Share on other sites
Quote:
Original post by TheUnbeliever
Quote:
Original post by anothrguitarist
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'?


*** 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]

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this