5
\$\begingroup\$

This code will return false if the long double rand begins with any string in the vector prefix.

bool prefix_vect(long double rand, std::vector<std::string> prefix) {
    std::string str_rand = std::to_string(rand);
    for(auto i : prefix) {
        if(boost::starts_with(str_rand, i))
            return false;
    }
    return true;
}

This code uses #include <boost/algorithm/string/predicate.hpp>.

How can I improve this code? Is there a way to make it fit into one line, such that it could be used without a function at all, and instead in an if-statement?

I am not experienced with C++ lambda expressions, is this something that could use them?

As a note: I don't care if it returns true or false, the Boolean can be switched if it makes this easier to optimize.

Edit: This isn't my main account, just a throwaway (unable to merge accounts, sadly).

\$\endgroup\$
  • \$\begingroup\$ I think you don't need boost for this. You could do the std::equal on the str_rand and beginning of i with the same length. It might require checking length of i. Also, is there a reason for not using references? \$\endgroup\$ – Incomputable Aug 22 '17 at 4:15
  • \$\begingroup\$ This function seems like an odd thing to do. What's the intended use case? \$\endgroup\$ – 200_success Aug 22 '17 at 4:43
  • \$\begingroup\$ @200_success I wrote a program that generates statistically random numbers. The program has a lot of neat (albeit useless) features, just for fun. One of the features is to only print the numbers if they begin with certain digits. So, for example, they could use the flag --prefix 0.1 0.3, and in the end it would print only the numbers beginning with 0.1 and 0.3 -- doing so by verifying it through this function. The numbers 0.1 and 0.3 are stored in a vector of strings, and the randomly generated numbers are long doubles, thus they need to be converted to a string to be compared easily. \$\endgroup\$ – XRFmVAeRTErelTbj Aug 22 '17 at 4:47
2
\$\begingroup\$

It's better to pass containers as references to avoid copying like this:

bool prefix_vect(long double rand, const std::vector<std::string>& prefix)

Also, when iterating on sequence, references also can improve performance:

for(const auto& i : prefix) {

As for one-liner I'd used something like this:

bool prefix_vect2(long double rand, const std::vector<std::string>& prefix) {
    std::string str_rand = std::to_string(rand);
    return std::none_of(prefix.begin(), prefix.end(),
             [&str_rand](const std::string& s){ return boost::starts_with(str_rand, s); });
}
\$\endgroup\$
0
\$\begingroup\$

If speed (when comparing many numbers against a constant set of prefixes) is your concern:

Converting a number to a decimal string representation is quite expensive. For the example given, a comparison x>=0.1 && x<0.2 does the same thing as the "0.1" prefix and is MUCH faster. Of course you need to convert the prefix strings to the interval limits, but only once for all test numbers.

If clarity of code is your concern, rename the function to begins_with_prefix() or similar, and otherwise leave it more or less the way it is. A call to a well-named function is always better understandable to a human reader than a line full of cryptic C++ constructs.

\$\endgroup\$
  • \$\begingroup\$ Your numeric approach won't work unless all numbers are of the same magnitude. Consider 10258 and the set of prefix strings {"103", "10"}. \$\endgroup\$ – Edward Sep 25 '17 at 12:48
  • \$\begingroup\$ @Edward You're correct. But the examples given by OP imply that there might be a common magnitude or a given decimal point, so the approach might be useful to the OP. \$\endgroup\$ – Ralf Kleberhoff Sep 25 '17 at 13:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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