# Project Euler #22 - Name scores

I have written code in C++ 11 and check output with Project Euler site, and it is correct. I am not showing output, just to keep it secret, at least from my end.

Please review my C++ 11 code.

#include <fstream>
#include <string>
#include <algorithm>
#include <iostream>
#include <vector>
#include <sstream>

using namespace std;

int main(int argc, char* argv[])
{

ifstream file("E:\\names.txt");

if (!file.is_open())
{
cout << "Unable to open file";
return -1;
}

/* Get all file data with ',' delimiter*/
vector<string> words;
string token;

while (getline(file, token, ','))
{
words.push_back(token);
}

//sort all the words
sort(words.begin(), words.end());

int counter = 0;
unsigned int grand_total = 0;

//get total of each word and multiply with it's position
for (auto it = words.begin(); it != words.end(); it++)
{
counter++;
//copy word from token excluding " "
string word((*it).begin() + 1, (*it).end() - 1);

int sub_total = 0;
for (string::iterator it = word.begin(); it != word.end(); it++)
{
//ascii value of A is 65
//A is 1, B is 2, so *it - 64
sub_total += *it - 64;
}

//grand total will be final answer
grand_total += sub_total*counter;
}

return 0;
}

-
Just a suggestion - might want to put a link to Euler Problem #22 in your question. Anyone can google it, but why not make it as easy as possible for those helping you? –  Dan Mar 27 at 17:06
@Dan Thanks Dan, that's a great suggestion. From next question I will follow it. –  pranitkothari Mar 27 at 17:11

The code is generally pretty clean, however, the major sticking point is that everything here is just shoved into main. Just breaking this up with a few functions that do one thing would be a good start:

std::vector<std::string> read_file(const std::string& path)
{
std::ifstream file(path);

if (!file.is_open())
{
std::cerr << "Unable to open file" << "\n";
std::exit(-1);
}

std::vector<string> words;
std::string token;

while (std::getline(file, token, ','))
{
words.push_back(token);
}

return words;
}


I've also put back all the std:: namespace qualifier. Using namespace std is something that is ok for very short programs like this, but is something you shouldn't get into the habit of doing.

How about a function to sum the values in a word?

int sum_word(const std::string& word)
{
constexpr static int ascii_to_integer = 64;
int total = 0;
for(auto c : word) {
total += (c - ascii_to_integer);
}
}


In fact, if you want to be fancy, this can be even shorter:

int sum_word(const std::string& word)
{
constexpr static int ascii_to_integer = 64;
return std::accumulate(word.begin(), word.end(), 0, [](int i) { return i - ascii_to_integer; });
}


Finally, we can have a function to encompass the outer loop (processing each word in the vector):

int total_word_position(const std::vector<std::string>& v)
{
int counter = 1;
int total = 0;

for(const std::string& s : v) {
// Exclude " "
std::string word(s.begin() + 1, s.end() - 1);
total += (sum_word(word) * counter);
++counter;
}
}


Your main method would then become:

int main()
{
std::sort(all_words.begin(), all_words.end());
int grand_total = total_word_position(all_words);
}


This has a number of benefits:

• Variable scope is minimized to a single smaller function, instead of a large main function.

• It's generally easier to read and debug, as there is less to keep in your head.

• It separates parts of the program that do very different things (reading a file vs sorting vs summing numbers).

-
+1. Really good advice. –  pranitkothari Mar 26 at 8:14
You forgot to use the path argument in read_file. :) –  João Portela Mar 26 at 11:37
@JoãoPortela Whoops, good catch. Fixed. –  Yuushi Mar 26 at 11:48

ifstream file("E:\\names.txt");

if (!file.is_open())

I believe if (!file) is a better way to do this since it checks if error bits are set.