6
\$\begingroup\$

I have a std::set<std::string> and I need to know the total number of characters (sum of all strings length):

static size_t SumLength(size_t value, const std::string &str)
{
    return str.length() + value;
}

size_t TotalLength(std::set<std::string> &stringSet)
{
   return std::accumulate(stringSet.begin(), stringSet.end(), 0, SumLength);
}

What do you think of this approach?

\$\endgroup\$

2 Answers 2

9
\$\begingroup\$

You should make the parameter to TotalLength const, so you can use it on constant sets. Since TotalLength does not modify its argument, there is no reason not to use a const reference.

Also this is a small thing, but I'd change the name of SumLength to AddLength as Sum to me suggests that it operates on multiple strings.

Other than that your code looks perfectly fine and straight forward.

\$\endgroup\$
1
  • \$\begingroup\$ Good catch about the const and nice comment about the function name. Thanks. \$\endgroup\$
    – bcsanches
    Mar 8, 2011 at 17:29
2
\$\begingroup\$

If you want to improve the locality of reference of the code you might replace SumLength with a lambda function. This would prevent future maintainers/etc from putting lots of code between SumLength and TotalLength. The drawback would be that you'd lose the ability to call SumLength by another function outside of TotalLength.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks, but no lambda on the current compiler that I have to use :(. \$\endgroup\$
    – bcsanches
    Dec 17, 2013 at 14:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.