From cd3c0c5f0be8f00e3083263895e3700a836ff692 Mon Sep 17 00:00:00 2001 From: Thomas Fussell Date: Sat, 30 Jul 2016 11:58:36 -0400 Subject: [PATCH] test number_formatter and improve exceptions --- include/xlnt/utils/exceptions.hpp | 9 --- source/detail/number_formatter.cpp | 67 +++++++++------------ source/detail/number_formatter.hpp | 21 +++---- source/styles/tests/test_number_format.hpp | 69 +++++++++++++++------- source/utils/exceptions.cpp | 5 -- 5 files changed, 85 insertions(+), 86 deletions(-) diff --git a/include/xlnt/utils/exceptions.hpp b/include/xlnt/utils/exceptions.hpp index 70a92ecd..bd800269 100644 --- a/include/xlnt/utils/exceptions.hpp +++ b/include/xlnt/utils/exceptions.hpp @@ -63,15 +63,6 @@ public: invalid_sheet_title(const std::string &title); }; -/// -/// Exception for incorrectly formatted named ranges. -/// -class XLNT_CLASS invalid_named_range : public exception -{ -public: - invalid_named_range(); -}; - /// /// Exception when a referenced number format is not in the stylesheet. /// diff --git a/source/detail/number_formatter.cpp b/source/detail/number_formatter.cpp index 52b9a9df..2d2a54ac 100644 --- a/source/detail/number_formatter.cpp +++ b/source/detail/number_formatter.cpp @@ -160,8 +160,6 @@ bool format_condition::satisfied_by(long double number) const return number < value; case condition_type::not_equal: return number != value; - default: - return false; } } @@ -199,24 +197,26 @@ void number_format_parser::parse() break; case number_format_token::token_type::color: - if (section.color != format_color::none - || section.condition.type != format_condition::condition_type::none - || section.locale != format_locale::none + if (section.has_color + || section.has_condition + || section.has_locale || !section.parts.empty()) { throw std::runtime_error("color should be the first part of a format"); } + section.has_color = true; section.color = color_from_string(token.string); break; case number_format_token::token_type::locale: { - if (section.locale != format_locale::none) + if (section.has_locale) { throw std::runtime_error("multiple locales"); } + section.has_locale = true; auto parsed_locale = locale_from_string(token.string); section.locale = parsed_locale.first; @@ -233,11 +233,12 @@ void number_format_parser::parse() case number_format_token::token_type::condition: { - if (section.condition.type != format_condition::condition_type::none) + if (section.has_condition) { throw std::runtime_error("multiple conditions"); } + section.has_condition = true; std::string value; if (token.string.front() == '<') @@ -439,9 +440,6 @@ void number_format_parser::parse() finalize(); return; - - default: - break; } token = parse_next_token(); @@ -524,6 +522,7 @@ void number_format_parser::finalize() fractional_seconds = true; } + //TODO this block needs improvement if (part.type == template_part::template_type::month_number || part.type == template_part::template_type::month_number_leading_zero) { @@ -552,8 +551,8 @@ void number_format_parser::finalize() if (previous.type == template_part::template_type::text && previous.string == ":" - && (before_previous.type == template_part::template_type::hour || - before_previous.type == template_part::template_type::hour_leading_zero)) + && (before_previous.type == template_part::template_type::hour_leading_zero + || before_previous.type == template_part::template_type::hour)) { fix = true; leading_zero = part.type == template_part::template_type::month_number_leading_zero; @@ -848,9 +847,9 @@ void number_format_parser::validate() if (codes_.size() > 2) { - if (codes_[0].condition.type != format_condition::condition_type::none && - codes_[1].condition.type != format_condition::condition_type::none && - codes_[2].condition.type != format_condition::condition_type::none) + if (codes_[0].has_condition + && codes_[1].has_condition + && codes_[2].has_condition) { throw std::runtime_error("format should have a maximum of two codes with conditions"); } @@ -1045,7 +1044,7 @@ number_formatter::number_formatter(const std::string &format_string, xlnt::calen std::string number_formatter::format_number(long double number) { - if (format_[0].condition.type != format_condition::condition_type::none) + if (format_[0].has_condition) { if (format_[0].condition.satisfied_by(number)) { @@ -1057,7 +1056,7 @@ std::string number_formatter::format_number(long double number) return std::string(11, '#'); } - if (format_[1].condition.type == format_condition::condition_type::none + if (!format_[1].has_condition || format_[1].condition.satisfied_by(number)) { return format_number(format_[1], number); @@ -1125,10 +1124,12 @@ std::string number_formatter::format_text(const std::string &text) std::string number_formatter::fill_placeholders(const format_placeholders &p, long double number) { + std::string result; + if (p.type == format_placeholders::placeholders_type::general || p.type == format_placeholders::placeholders_type::text) { - auto result = std::to_string(number); + result = std::to_string(number); while (result.back() == '0') { @@ -1155,13 +1156,11 @@ std::string number_formatter::fill_placeholders(const format_placeholders &p, lo auto integer_part = static_cast(number); - switch (p.type) + if (p.type == format_placeholders::placeholders_type::integer_only + || p.type == format_placeholders::placeholders_type::integer_part + || p.type == format_placeholders::placeholders_type::fraction_integer) { - case format_placeholders::placeholders_type::integer_only: - case format_placeholders::placeholders_type::integer_part: - case format_placeholders::placeholders_type::fraction_integer: - { - auto result = std::to_string(integer_part); + result = std::to_string(integer_part); while (result.size() < p.num_zeros) { @@ -1195,16 +1194,13 @@ std::string number_formatter::fill_placeholders(const format_placeholders &p, lo { result.push_back('%'); } - - return result; } - - case format_placeholders::placeholders_type::fractional_part: + else if (p.type == format_placeholders::placeholders_type::fractional_part) { auto fractional_part = number - integer_part; - auto result = fractional_part == 0 ? std::string(".") : std::to_string(fractional_part).substr(1); + result = fractional_part == 0 ? std::string(".") : std::to_string(fractional_part).substr(1); - while (result.back() == '0' || result.size() > (p.num_zeros + p.num_optionals + 1)) + while (result.back() == '0' || result.size() > (p.num_zeros + p.num_optionals + p.num_spaces + 1)) { result.pop_back(); } @@ -1214,7 +1210,7 @@ std::string number_formatter::fill_placeholders(const format_placeholders &p, lo result.push_back('0'); } - while (result.size() < p.num_zeros + p.num_spaces + 1) + while (result.size() < p.num_zeros + p.num_optionals + p.num_spaces + 1) { result.push_back(' '); } @@ -1223,13 +1219,9 @@ std::string number_formatter::fill_placeholders(const format_placeholders &p, lo { result.push_back('%'); } - - return result; } - default: - return ""; - } + return result; } std::string number_formatter::fill_scientific_placeholders(const format_placeholders &integer_part, @@ -1591,9 +1583,6 @@ std::string number_formatter::format_number(const format_code &format, long doub case template_part::template_type::day_name: result.append(day_names->at(dt.weekday() - 1)); break; - - case template_part::template_type::bad: - throw std::runtime_error("bad format"); } } diff --git a/source/detail/number_formatter.hpp b/source/detail/number_formatter.hpp index c9be063b..ff2428a8 100644 --- a/source/detail/number_formatter.hpp +++ b/source/detail/number_formatter.hpp @@ -36,7 +36,6 @@ namespace detail { enum class format_color { - none, black, blue, cyan, @@ -105,7 +104,6 @@ enum class format_color enum class format_locale { - none = 0, arabic_saudi_arabia = 0x401, bulgarian = 0x402, catalan = 0x403, @@ -218,14 +216,13 @@ struct XLNT_CLASS format_condition { enum class condition_type { - none, less_than, less_or_equal, equal, not_equal, greater_than, greater_or_equal - } type = condition_type::none; + } type = condition_type::not_equal; long double value = 0; @@ -236,7 +233,6 @@ struct format_placeholders { enum class placeholders_type { - bad, general, text, integer_only, @@ -248,7 +244,7 @@ struct format_placeholders scientific_significand, scientific_exponent_plus, scientific_exponent_minus - } type = placeholders_type::bad; + } type = placeholders_type::general; bool use_comma_separator = false; bool percentage = false; @@ -264,7 +260,6 @@ struct number_format_token { enum class token_type { - bad, color, locale, condition, @@ -275,7 +270,7 @@ struct number_format_token datetime, end_section, end - } type = token_type::bad; + } type = token_type::end; std::string string; }; @@ -284,7 +279,6 @@ struct template_part { enum class template_type { - bad, text, fill, space, @@ -313,7 +307,7 @@ struct template_part elapsed_hours, elapsed_minutes, elapsed_seconds - } type = template_type::bad; + } type = template_type::general; std::string string; format_placeholders placeholders; @@ -321,8 +315,11 @@ struct template_part struct format_code { - format_color color = format_color::none; - format_locale locale = format_locale::none; + bool has_color = false; + format_color color = format_color::black; + bool has_locale = false; + format_locale locale = format_locale::english_united_states; + bool has_condition = false; format_condition condition; bool is_datetime = false; bool is_timedelta = false; diff --git a/source/styles/tests/test_number_format.hpp b/source/styles/tests/test_number_format.hpp index df365327..d0c2d1ee 100644 --- a/source/styles/tests/test_number_format.hpp +++ b/source/styles/tests/test_number_format.hpp @@ -1,11 +1,10 @@ #pragma once #include +#include #include -#include "pugixml.hpp" #include -#include class test_number_format : public CxxTest::TestSuite { @@ -375,16 +374,6 @@ public: TS_ASSERT_EQUALS(formatted, "text"); } - void test_bad_part() - { - xlnt::detail::template_part bad_part; - xlnt::detail::format_code bad_code; - bad_code.parts.push_back(bad_part); - xlnt::detail::number_formatter formatter("", xlnt::calendar::windows_1900); - formatter.format_ = { bad_code }; - TS_ASSERT_THROWS(formatter.format_number(1), std::runtime_error); - } - void test_conditional_format() { xlnt::number_format nf; @@ -487,7 +476,53 @@ public: formatted = nf.format(6, xlnt::calendar::windows_1900); TS_ASSERT_EQUALS(formatted, "a---------b"); } - + + void test_placeholders_zero() + { + xlnt::number_format nf; + nf.set_format_string("00"); + auto formatted = nf.format(6, xlnt::calendar::windows_1900); + TS_ASSERT_EQUALS(formatted, "06"); + + nf.set_format_string("00"); + formatted = nf.format(63, xlnt::calendar::windows_1900); + TS_ASSERT_EQUALS(formatted, "63"); + } + + void test_placeholders_space() + { + xlnt::number_format nf; + nf.set_format_string("?0"); + auto formatted = nf.format(6, xlnt::calendar::windows_1900); + TS_ASSERT_EQUALS(formatted, " 6"); + + nf.set_format_string("?0"); + formatted = nf.format(63, xlnt::calendar::windows_1900); + TS_ASSERT_EQUALS(formatted, "63"); + + nf.set_format_string("?0"); + formatted = nf.format(637, xlnt::calendar::windows_1900); + TS_ASSERT_EQUALS(formatted, "637"); + + nf.set_format_string("0.?"); + formatted = nf.format(6, xlnt::calendar::windows_1900); + TS_ASSERT_EQUALS(formatted, "6. "); + + nf.set_format_string("0.0?"); + formatted = nf.format(6.3, xlnt::calendar::windows_1900); + TS_ASSERT_EQUALS(formatted, "6.3 "); + formatted = nf.format(6.34, xlnt::calendar::windows_1900); + TS_ASSERT_EQUALS(formatted, "6.34"); + } + + void test_scientific() + { + xlnt::number_format nf; + nf.set_format_string("0E-0"); + auto formatted = nf.format(6.1, xlnt::calendar::windows_1900); + TS_ASSERT_EQUALS(formatted, "6.1E0"); + } + void test_locale_currency() { xlnt::number_format nf; @@ -627,14 +662,6 @@ public: TS_ASSERT_THROWS(nf.format(1.2, xlnt::calendar::windows_1900), std::runtime_error); } - void test_none_condition() - { - xlnt::detail::format_condition f; - f.type = xlnt::detail::format_condition::condition_type::none; - f.value = 3; - TS_ASSERT(!f.satisfied_by(3)); - } - void format_and_test(const xlnt::number_format &nf, const std::array &expect) { long double positive = 42503.1234; diff --git a/source/utils/exceptions.cpp b/source/utils/exceptions.cpp index 50eea8cf..0e147de9 100644 --- a/source/utils/exceptions.cpp +++ b/source/utils/exceptions.cpp @@ -52,11 +52,6 @@ invalid_data_type::invalid_data_type() { } -invalid_named_range::invalid_named_range() - : exception("named range not found or not owned by this worksheet") -{ -} - invalid_file::invalid_file(const std::string &filename) : exception(std::string("couldn't open file: (") + filename + ")") {