Project Euler 22: Names scores by Loki 2

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";
}


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.

• Inserting a single char instead of a length-1 string Not sure what this is referring to? May 3, 2017 at 19:16
• Well spotted that copy of names in the for loop. May 3, 2017 at 19:17
• Its lucky that list had no duplicates. I would have never spotted that kind of bug quickly. May 3, 2017 at 19:18
• Your last line is std::cout << score << "\n"; instead of the marginally more efficient std::cout << score << '\n';. May 3, 2017 at 19:52
• OK. But, I don't believe that is a measurable difference. May 3, 2017 at 20:15

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!

• An dissenting view from me - short names are fine (nay, better) in single-line scopes such as the lambda. Use long, descriptive names for things you can't expect to hold in you (wetware) working set, but compact names for things that are used in every line of their scope. May 3, 2017 at 7:45
• Actually, I might go with [](long sum, char c) so that it's clear where the first argument comes from. May 3, 2017 at 7:58
• That's fair. To be honest, I find lambdas and blocks and other inline constructs really hard to read at all. So for me longer names are still a good idea. But if you're comfortable with the syntax I can see why you might prefer shorter names. May 3, 2017 at 16:24