Ok, so I tried almost everything mentioned although finally I didn't realize what's the use of templates as the input of the name must always be a string. Anyway, of course you can use templates if you want but I don't see the use for this particular program. It just gets more complicated for the programmer. Do we gain more speed as the template executes each time it's called?
I believe alvaro was not talking about the C++ language feature when they said "template", but rather that the string featuring replacement sequences acts as a kind of "template string".
Also can you elaborate a little what OP is?
OP generally stands for "Original Poster".
Now, I want to ask is there any other way to optimize the code and make it faster/better/more readable/more efficient?
My comments on your current source are:
- The string constants ("name", "/NAME/" (etc) and the number of constants 6 is hard coded in a number of places.
- You have code duplication in the loop body of enterDetails
- Your replacement loop doesn't leverage the algorithmic gains that alvaro hinted were possible.
- Don't abuse static here.
In particular, for number 3 your solution is probably less efficient - you spend time building and iterating the map, but the algorithm is unchanged. The result is more work being done.
Here is my attempt at fixing some of these flaws:
#include <iostream>
#include <cstdio>
#include <cstdlib>
#include <string>
#include <fstream>
#include <sstream>
#include <map>
#include <stdexcept>
#include <iterator>
using namespace std;
const char Placeholder = '/';
const string PlayerDetails[] = {
"name", "age", "weight", "class", "race", "hair color", "pant size"
};
const unsigned NumDetails = sizeof(PlayerDetails) / sizeof(PlayerDetails[0]);
typedef map<string, string> PlayerInfo;
string makePlaceholder(const string &attribute) {
string result;
result.push_back(Placeholder);
for(string::size_type i = 0 ; i < attribute.size() ; ++i) {
char c = attribute[i];
if(c == ' ') {
c = '-';
} else {
c = toupper(c);
}
result.push_back(c);
}
result.push_back(Placeholder);
return result;
}
PlayerInfo enterDetails(istream &input, ostream &output) {
PlayerInfo result;
for(unsigned i = 0 ; i < NumDetails ; ++i) {
const string &attribute = PlayerDetails[i];
output << "Please enter " << attribute << ": ";
string line;
getline(input, line);
string placeholder = makePlaceholder(attribute);
result.insert(make_pair(placeholder, line));
}
return result;
}
string readFile(const string &filename) {
fstream stream(filename.c_str());
if(!stream) {
throw runtime_error("Failed to open: " + filename);
}
return string((istreambuf_iterator<char>(stream)), istreambuf_iterator<char>());
}
void replaceTags(string &tagged, const PlayerInfo &player) {
string::size_type begin = 0;
while( (begin = tagged.find(Placeholder, begin)) != string::npos) {
string::size_type end = tagged.find(Placeholder, begin + 1);
if(end == string::npos) {
stringstream error;
error << "No matching " << Placeholder << " at position " << begin;
throw runtime_error(error.str());
}
int length = (end - begin) + 1;
string tag = tagged.substr(begin, length);
PlayerInfo::const_iterator it = player.find(tag);
if(it == player.end()) {
stringstream error;
error << "Unknown tag: '" << tag << "' between " << begin << " and " << end;
throw runtime_error(error.str());
}
tagged.replace(begin, length, it->second);
}
}
int main() {
PlayerInfo player = enterDetails(cin, cout);
// Show player information (debug)
for(PlayerInfo::iterator i = player.begin() ; i != player.end() ; ++i) {
cout << i->first << " -> " << i->second << '\n';
}
cout << "\n---------\n";
string tagged = readFile("Rebels_after_start.txt");
cout << tagged;
std::cout << "\n---------\n";
replaceTags(tagged, player);
cout << tagged;
std::cout << "\n---------\n";
}
Note that I have one definitive list of string constants. All other parts of the program infer information from this. So the user prompts use it, the /PLACEHOLDER/ values are built from these strings (uppercased with spaces replaced with hyphens). Even the number of constants is inferred from the size of the array.
Essentially you can now add or remove player attributes by making a single change to that array.
You can ignore the implementation of readFile() if you like. Essentially it gets "iterators" for the file, and constructs a string from these iterators. The result is the contents of the file in a string.
Perhaps the most interesting part is the implementation of replaceTags. It is
relatively efficient - it only makes a single pass over the input string. It looks for patterns of two forward slashes, and attempts to determine the replacement using the player map. Unknown tags or cases where only a single forward slash is found are handled via exceptions.
One important gain is that the player attribute map is built once. If we were displaying reams of text to the user, we can keep re-using the same mapping over and over.
However, the result is that the code is becoming less clear and harder to read. Some of Servant Of The Lords earlier examples were better from this point of view, it is immediately clear what this code wants to do:
text = ReplaceAll(text, "/NAME/", playerName);
text = ReplaceAll(text, "/AGE/", playerAge);
text = ReplaceAll(text, "/WEIGHT/", playerAge);
text = ReplaceAll(text, "/CLASS/", playerClass);
text = ReplaceAll(text, "/RACE/", playerRace);
text = ReplaceAll(text, "/HAIR-COLOR/", playerHairColor);
text = ReplaceAll(text, "/PANT-SIZE/", playerPantSize);
In the above code, it is less clear that this is going to happen.
2
