3
\$\begingroup\$

Having mostly used Qt and its classes like QString, I wrote a little exercise with plain C++11. I'm looking for two kinds of input: generally improving the code and/or doing it better in an entirely different way (still standard C++11 or C++14, no Boost or anything).

The code is for printing doubles with given number of significant digits, by first producing a string using scientific notation, and then manipulating that string to move the dot and pad it with zeros.

#include <sstream>
#include <iostream>
#include <iomanip>
#include <vector>
#include <algorithm>
#include <stdexcept>

template <typename T>
std::string number_to_sci_string(T number, int precision) {
    std::stringstream ss;
    ss << std::scientific << std::setprecision(precision-1) << number;
    return ss.str();
}

std::string sci_string_to_normal(const std::string &s) {
    auto dot_pos = s.find('.');
    auto e_pos = s.find('e');
    if (dot_pos == std::string::npos || // dot required
        dot_pos < 1 ||  // part1 below must not be empty
        e_pos == std::string::npos || // e required
        e_pos >= s.length()-1 || // must not be last
        e_pos - dot_pos < 2) { // part2 below must not be empty
        throw std::invalid_argument(s);
    }

    auto exponent = std::stoi(s.substr(e_pos + 1), nullptr, 10);
    if (exponent == 0) {
        return s.substr(0, e_pos);
    }
    else {
        auto part1 = s.substr(0, dot_pos);
        auto part2 = s.substr(dot_pos + 1, e_pos - dot_pos - 1);
        auto part2length = static_cast<ssize_t>(part2.length());
        if (exponent >= part2length) {
            auto fill = std::string(exponent - part2length, '0');
            return part1 + part2 + fill;
        }
        else if (exponent < 0) {
            auto fill = "0." + std::string(-1 - exponent, '0');
            // this is the only case where sign matters
            auto sign = (s[0] == '-' || s[0] == '+')
                        ? std::string(1, s[0])
                        : std::string();
            return sign + fill + part1.substr(sign.length()) + part2;
        }
        else { // 0 < exponent < part2.length()
            return part1 + part2.substr(0,  exponent) + "." + part2.substr(exponent);
        }
    }
}

using namespace std;

int main(void)
{
    vector<double> nums{ -2000.0005, -1.2345234, -0.000011345, 0, 0.0000001, 0.1342134, 1.1234, 10000 };
    for_each(nums.begin(), nums.end(), [](double &n) {
        string sci = number_to_sci_string(n, 5);
        string nor = sci_string_to_normal(sci);
        cout << n << ": " << sci << " == " << nor << endl;
    });
}
\$\endgroup\$
3
\$\begingroup\$
  1. number_to_sci_string looks ok, though you could considerably improve performance by chucking the template, restricting to float, double and long double and using snprintf:

    std::string number_to_sci_string(long double number, int precision) {
        std::string s(std::snprintf(0, 0, "%.*Le", precision-1, number), '\0');
        std::snprintf(&s[0], s.size()+1, "%.*Le", precision-1, number);
        return s;
    }
    

    Also, I really wonder why you are using a different notion of precision than the standard library...

  2. I suggest you be a bit more flexible in what you accept:

    • The exponent e00 should be optional.
    • The fractional part (and decimal point) .00 should be optional.
    • If there's a fractional part, the integer part should be optional.
  3. Don't use using namespace std;, even in the implementation-file: You don't control what symbols it contains, and there are no guarantees none are added.

  4. The opening brace of main()'s body is, in contrast to all other functions, on a separate line. Why?

  5. Prefer the for-range-loop to std::for_each+lambda. The compiler should compile both to the same result, but the former is simpler and looks better.

\$\endgroup\$
  • 1
    \$\begingroup\$ I pretty much agree with all points (except 3 only partly: I agree with the point you're making, but I consider it a trade-off which is sometimes worth it). Answer to your "why" questions is basically, by accident or due to carelessness (which is why code review is a good thing). \$\endgroup\$ – hyde Nov 12 '15 at 17:27

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.