From b6ea8dcc522c98f3d4b8bab0ca0ee459803050c2 Mon Sep 17 00:00:00 2001 From: Marko Zivanovic Date: Fri, 9 Oct 2015 23:30:19 +0200 Subject: [PATCH] Reduce code complexity by moving to data-driven algorithms. --- src/Semver200_comparator.cpp | 68 ++++++++++++++++++++++++------------ src/Semver200_parser.cpp | 24 ++++++++----- 2 files changed, 62 insertions(+), 30 deletions(-) diff --git a/src/Semver200_comparator.cpp b/src/Semver200_comparator.cpp index 2087e4e..6e0437a 100644 --- a/src/Semver200_comparator.cpp +++ b/src/Semver200_comparator.cpp @@ -23,6 +23,8 @@ SOFTWARE. */ #include +#include +#include #include "version.h" using namespace std; @@ -31,6 +33,7 @@ namespace { using namespace version; + // Compare normal version identifiers. int compare_normal(const Version_data& l, const Version_data& r) { if (l.major > r.major) return 1; if (l.major < r.major) return -1; @@ -41,46 +44,67 @@ namespace { return 0; } - int compare_prerel_identifiers(const Prerelease_identifier& l, const Prerelease_identifier& r) { - if (l.second == Identifier_type::alnum && r.second == Identifier_type::alnum) { - auto cmp = l.first.compare(r.first); - if (cmp == 0) return cmp; + // Compare alphanumeric prerelease identifiers. + inline int cmp_alnum_prerel_ids(const string& l, const string& r) { + auto cmp = l.compare(r); + if (cmp == 0) { + return cmp; + } else { return cmp > 0 ? 1 : -1; - } else if (l.second == Identifier_type::alnum && r.second == Identifier_type::num) { - return 1; - } else if (l.second == Identifier_type::num && r.second == Identifier_type::alnum) { - return -1; - } else if (l.second == Identifier_type::num && r.second == Identifier_type::num) { - int li = stoi(l.first); - int ri = stoi(r.first); - if (li == ri) return 0; - return li > ri ? 1 : -1; } - throw logic_error("unexpected identifier types: " + to_string(static_cast(l.second)) + ", " + - to_string(static_cast(r.second))); + } + + // Compare numeric prerelease identifiers. + inline int cmp_num_prerel_ids(const string& l, const string& r) { + int li = stoi(l); + int ri = stoi(r); + if (li == ri) return 0; + return li > ri ? 1 : -1; + } + + using Prerel_types = pair; + using Prerel_id_comparator = function; + const map comparators = { + { { Identifier_type::alnum, Identifier_type::alnum }, cmp_alnum_prerel_ids }, + { { Identifier_type::alnum, Identifier_type::num }, [](const string&, const string&) {return 1;} }, + { { Identifier_type::num, Identifier_type::alnum }, [](const string&, const string&) {return -1;} }, + { { Identifier_type::num, Identifier_type::num }, cmp_num_prerel_ids } + }; + + // Compare prerelease identifiers based on their types. + int compare_prerel_identifiers(const Prerelease_identifier& l, const Prerelease_identifier& r) { + auto cmp = comparators.at({ l.second, r.second }); + return cmp(l.first, r.first); + } + + inline int cmp_rel_prerel(const Prerelease_identifiers& l, const Prerelease_identifiers& r) { + if (l.empty() && !r.empty()) return 1; + if (r.empty() && !l.empty()) return -1; + return 0; } } namespace version { int Semver200_comparator::compare(const Version_data& l, const Version_data& r) const { + // Compare normal version components. int cmp = compare_normal(l, r); if (cmp != 0) return cmp; - // release version is always higher than prerelease - if (l.prerelease_ids.empty() && !r.prerelease_ids.empty()) return 1; - if (r.prerelease_ids.empty() && !l.prerelease_ids.empty()) return -1; + // Compare if one version is release and the other prerelease; release is always higher than prerelease. + cmp = cmp_rel_prerel(l.prerelease_ids, r.prerelease_ids); + if (cmp != 0) return cmp; - // compare prerelease by looking at each identifier: numeric ones are compared as numbers, - // alphanum as ASCII strings + // Compare prerelease by looking at each identifier: numeric ones are compared as numbers, + // alphanum as ASCII strings. auto shorter = min(l.prerelease_ids.size(), r.prerelease_ids.size()); for (size_t i = 0; i < shorter; i++) { cmp = compare_prerel_identifiers(l.prerelease_ids[i], r.prerelease_ids[i]); if (cmp != 0) return cmp; } - // prerels are the same, to the length of the shorter one; - // if they are the same length, then versions are equal, otherwise, longer wins + // Prerelease identifiers are the same, to the length of the shorter version string; + // if they are the same length, then versions are equal, otherwise, longer one wins. if (l.prerelease_ids.size() == r.prerelease_ids.size()) return 0; return l.prerelease_ids.size() > r.prerelease_ids.size() ? 1 : -1; } diff --git a/src/Semver200_parser.cpp b/src/Semver200_parser.cpp index cee6d72..634ba23 100644 --- a/src/Semver200_parser.cpp +++ b/src/Semver200_parser.cpp @@ -35,23 +35,28 @@ namespace { using Validator = function; using State_transition_hook = function; - /// State transition is described by a character that triggers it, a cstate to transition to and + /// State transition is described by a character that triggers it, a state to transition to and /// optional hook to be invoked on transition. using Transition = tuple; using Transitions = vector; using State = tuple; using State_machine = map; + // Ranges of characters allowed in prerelease and build identifiers. + const vector> allowed_prerel_id_chars = { + { '0', '9' },{ 'A','Z' },{ 'a','z' },{ '.','.' },{ '-','-' } + }; + inline Transition mkx(const char c, Parser_state p, State_transition_hook pth) { return make_tuple(c, p, pth); } - /// Advance parser cstate machine by a single step. + /// Advance parser state machine by a single step. /** - Perform single step of parser cstate machine: if character matches one from transition tables - - trigger transition to next cstate; otherwise, validate if current token is in legal cstate + Perform single step of parser state machine: if character matches one from transition tables - + trigger transition to next state; otherwise, validate if current token is in legal state (throw Parse_error if not) and then add character to current token; State transition includes - preparing various vars for next cstate and invoking cstate transition hook (if specified) which is + preparing various vars for next state and invoking state transition hook (if specified) which is where whole tokens are validated. */ inline void process_char(const char c, Parser_state& cstate, Parser_state& pstate, @@ -74,9 +79,13 @@ namespace { if (tgt.compare(0, 1, "0") == 0) throw version::Parse_error("leading 0 not allowed"); } - /// Validate prerelease and build version components. + /// Validate that prerelease and build version identifiers are comprised of allowed chars only. inline void prerelease_version_validator(const string&, const char c) { - if ((c < '0' || c > '9') && (c < 'A' || c > 'Z') && (c < 'a' || c > 'z') && c != '.' && c != '-') + bool res = false; + for (const auto& r : allowed_prerel_id_chars) { + res |= (c >= r.first && c <= r.second); + } + if (!res) throw version::Parse_error("invalid character encountered: " + string(1, c)); } @@ -113,7 +122,6 @@ namespace { id.clear(); } - } namespace version {