Project Euler 22: Names scores by Loki 2

Question

Alternative to Project Euler 22 solution.

Project Euler 22

Slightly more brittle than the original. This code depends on the input stream being exactly as specified. But on the other side of the coin it's slightly easier to read because you don't have to wonder about std::locale and code that uses facets like std::ctype<> (which few people understand how to use).

Using names.txt (right click and 'Save Link/Target As...'), a 46K text file containing over five-thousand first names, begin by sorting it into alphabetical order. Then working out the alphabetical value for each name, multiply this value by its alphabetical position in the list to obtain a name score.

For example, when the list is sorted into alphabetical order, COLIN, which is worth 3 + 15 + 12 + 9 + 14 = 53, is the 938th name in the list. So, COLIN would obtain a score of 938 × 53 = 49714.

What is the total of all the name scores in the file?

#include <iostream>
#include <fstream>
#include <iterator>
#include <string>
#include <set>
#include <numeric>

long scoreName(std::string const& name)
{
    return std::accumulate(std::begin(name), std::end(name), 0L, 
                           [](long v1, char x){return v1 + x - 'A' + 1;});
}

struct FirstName
{
    // Read a name from a stream.
    // Each name is quoted and separated with comma.
    // There are no extra spaces on the stream.
    //  Sample: "DEBBIE","APRIL","LESLIE","CLARA","LUCILLE","JAMIE"
    std::string name;

    // Read the next name from the input.
    // Note does not remove quotes.
    friend std::istream& operator>>(std::istream& s, FirstName& data)
    {
        return std::getline(s, data.name, ',');
    }

    // When we convert this object to a std::string
    // we remove the quotes around the name. In the code
    // below this happens as the iterator is de-reference into the set.
    operator std::string() const
    {
        return name.substr(1, name.size() - 2);
    }
};

Thus leaving a very simple main.

int main()
{
    std::ifstream   data("euler/e22.data");

    std::set<std::string> names{std::istream_iterator<FirstName>(data), 
                                std::istream_iterator<FirstName>()};

    long    score   = 0;
    long    loop    = 1;
    for(auto name: names) {
        score += (loop * scoreName(name));
        ++loop;
    }
    std::cout << score << "\n";
}

Answer 1

1. std::accumulate with a lambda is quite a bit more complicated than writing out the range-for-loop manually, and it isn't even more concise:

   long result = 0;
   for(char c : name)
       result += c - 'A' + 1;
   return result;
   

2. Using a std::set instead of a std::vector and sorting it once manually after reading everything in is possibly a bit less efficient. The really interesting part though is that duplicates are silently discarded, and the problem-statement doesn't seem to allow that.

3. Your range-for-loop in main needlessly copies every string. Better go for references even though that's slightly longer:

   for(const auto& name: names)
   

4. Inserting a single char instead of a length-1 string is probably slightly more efficient.

Answer 2

This is very straightforward and easy to understand. I don't have a lot to say about it! The main thing I see that could be improved is the same thing that can be improved in almost every program - the naming. Taking it from the top:

The parameters in the lambda for std::accumulate are meaningless. v1 would be better named accumulator or just sum or better yet letterSum. x should have a clearer name like nextCharacter or something along those lines.

Likewise in your operator>>, the argument data should be called something like nextName. Any time I see the generic word data in my code, I try to come up with a more meaningful name. (Same with info, record, etc.) Which is why I think you should also rename the local variable data in main() to names, or dictionary, or nameList, or something like that.

The variable loop is not a loop and does not control a loop. A name like index would be more accurate, and even better would be nameIndex or at least lineIndex since it's keeping track of names, or lines read from the file. (Well, technically not lines since they're comma separated, but you get the idea.)

That said, I wish I saw more code like this on a daily basis!