HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

Project Euler #22 - Name scores

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
projectnameeulerscores

Problem

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 
#include 
#include 
#include 
#include 
#include 

using namespace std;

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

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

    if (!file.is_open())
    {
        cout  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;
}

Solution

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 read_file(const std::string& path)
{
    std::ifstream file(path);

    if (!file.is_open())
    {
        std::cerr  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);
    }
    return total;
}


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& 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;
    }
    return total;
}


Your main method would then become:

int main()
{
    std::vector all_words = read_file("E:\\names.txt");
    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).

Code Snippets

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;
}
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);
    }
    return total;
}
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; });
}
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;
    }
    return total;
}
int main()
{
    std::vector<std::string> all_words = read_file("E:\\names.txt");
    std::sort(all_words.begin(), all_words.end());
    int grand_total = total_word_position(all_words);
}

Context

StackExchange Code Review Q#45377, answer score: 10

Revisions (0)

No revisions yet.