From a6df0746b8b6fe9e417d9242f2f7feda956d9894 Mon Sep 17 00:00:00 2001 From: Sergey Magidovich Date: Tue, 22 Dec 2015 14:13:51 +0300 Subject: [PATCH] Code Review. --- 3party/opening_hours/rules_evaluation.cpp | 1 + base/base_tests/enumerate_test.cpp | 8 ++-- base/base_tests/range_iterator_test.cpp | 8 ++-- base/enumerate.hpp | 50 +++++++++++------------ base/range_iterator.hpp | 8 ++-- editor/ui2oh.cpp | 2 +- indexer/feature.cpp | 6 +-- 7 files changed, 41 insertions(+), 42 deletions(-) diff --git a/3party/opening_hours/rules_evaluation.cpp b/3party/opening_hours/rules_evaluation.cpp index 1ea5490e73..ea266c7e3d 100644 --- a/3party/opening_hours/rules_evaluation.cpp +++ b/3party/opening_hours/rules_evaluation.cpp @@ -196,6 +196,7 @@ std::tm MakeTimetuple(time_t const timestamp) namespace osmoh { +// ADL shadows ::operator==. using ::operator==; bool IsActive(Timespan const & span, std::tm const & time) diff --git a/base/base_tests/enumerate_test.cpp b/base/base_tests/enumerate_test.cpp index 77c608b6b5..5bd0d09825 100644 --- a/base/base_tests/enumerate_test.cpp +++ b/base/base_tests/enumerate_test.cpp @@ -9,14 +9,14 @@ UNIT_TEST(enumerate) { { map result; - for (auto const p : my::enumerate(std::vector{1, 2, 3})) + for (auto const p : my::Enumerate(std::vector{1, 2, 3})) result.insert(p); TEST_EQUAL(result, (map{{0, 1}, {1, 2}, {2, 3}}), ()); } { map result; - for (auto const p : my::enumerate(std::vector{1, 2, 3}, 10)) + for (auto const p : my::Enumerate(std::vector{1, 2, 3}, 10)) result.insert(p); TEST_EQUAL(result, (map{{10, 1}, {11, 2}, {12, 3}}), ()); } @@ -24,14 +24,14 @@ UNIT_TEST(enumerate) { std::vector const vec{1, 2, 3}; map result; - for (auto const p : my::enumerate(vec)) + for (auto const p : my::Enumerate(vec)) result.insert(p); TEST_EQUAL(result, (map{{0, 1}, {1, 2}, {2, 3}}), ()); } { std::vector vec{1, 2, 3, 4, 5, 6}; - for (auto const p : my::enumerate(vec, -6)) + for (auto const p : my::Enumerate(vec, -6)) p.item *= p.index; TEST_EQUAL(vec, (std::vector{-6, -10, -12, -12, -10, -6}), ()); diff --git a/base/base_tests/range_iterator_test.cpp b/base/base_tests/range_iterator_test.cpp index 5efc3a0c8c..f99301af8b 100644 --- a/base/base_tests/range_iterator_test.cpp +++ b/base/base_tests/range_iterator_test.cpp @@ -10,28 +10,28 @@ UNIT_TEST(RangeIterator) { vector result; - for (auto const i : range(5)) + for (auto const i : Range(5)) result.push_back(i); TEST_EQUAL(result, (vector{0, 1, 2, 3, 4}), ()); } { vector result; - for (auto const i : range(2, 5)) + for (auto const i : Range(2, 5)) result.push_back(i); TEST_EQUAL(result, (vector{2, 3, 4}), ()); } { vector result; - for (auto const i : reverse_range(5)) + for (auto const i : ReverseRange(5)) result.push_back(i); TEST_EQUAL(result, (vector{4, 3, 2, 1, 0}), ()); } { vector result; - for (auto const i : reverse_range(2, 5)) + for (auto const i : ReverseRange(2, 5)) result.push_back(i); TEST_EQUAL(result, (vector{4, 3, 2}), ()); } diff --git a/base/enumerate.hpp b/base/enumerate.hpp index eaf3b61ad3..60461beb3b 100644 --- a/base/enumerate.hpp +++ b/base/enumerate.hpp @@ -8,53 +8,51 @@ namespace my namespace details { template -struct const_respective_iterator +struct ConstRespectiveIterator { - using hint = int; using type = typename TCollection::iterator; }; template -struct const_respective_iterator +struct ConstRespectiveIterator { - using hint = double; using type = typename TCollection::const_iterator; }; template -using const_respective_iterator_t = - typename const_respective_iterator::type>::type; +using ConstRespectiveIteratorT = + typename ConstRespectiveIterator::type>::type; } // namespace details -template -struct IndexedElement : std::pair +template +struct IndexedElement : std::pair { - using base = std::pair; - using base::base; + using TBase = std::pair; + using TBase::TBase; - TCounter const index{base::first}; - TElement & item{base::second}; + TIndex const index{TBase::first}; + TElement & item{TBase::second}; }; -template -IndexedElement MakeIndexedElement(TElement && item, TCounter counter = {}) +template +IndexedElement MakeIndexedElement(TElement && item, TIndex counter = {}) { - return IndexedElement(counter, item); + return IndexedElement(counter, item); } -template +template struct EnumeratingIterator { using original_iterator = TIterator; using original_reference = typename std::iterator_traits::reference; using difference_type = typename std::iterator_traits::difference_type; - using value_type = IndexedElement; + using value_type = IndexedElement; using pointer = value_type *; using reference = value_type &; using iterator_category = std::forward_iterator_tag; - EnumeratingIterator(original_iterator const it, TCounter const counter) + EnumeratingIterator(original_iterator const it, TIndex const counter) : m_iterator(it), m_counter(counter) { } @@ -70,18 +68,18 @@ struct EnumeratingIterator bool operator==(EnumeratingIterator const & it) const { return m_iterator == it.m_iterator; } bool operator!=(EnumeratingIterator const & it) const { return !(*this == it); } original_iterator m_iterator; - TCounter m_counter; + TIndex m_counter; }; -template +template struct EnumeratorWrapper { using original_iterator = TIterator; - using iterator = EnumeratingIterator; + using iterator = EnumeratingIterator; using value_type = typename std::iterator_traits::value_type; EnumeratorWrapper(original_iterator const begin, original_iterator const end, - TCounter const countFrom) + TIndex const countFrom) : m_begin(begin), m_end(end), m_countFrom(countFrom) { } @@ -90,12 +88,12 @@ struct EnumeratorWrapper iterator end() { return {m_end, {}}; } original_iterator const m_begin; original_iterator const m_end; - TCounter const m_countFrom; + TIndex const m_countFrom; }; -template -auto enumerate(TCollection && collection, TCounter const counter = {}) - -> EnumeratorWrapper, TCounter> +template +auto Enumerate(TCollection && collection, TIndex const counter = {}) + -> EnumeratorWrapper, TIndex> { return {begin(collection), end(collection), counter}; } diff --git a/base/range_iterator.hpp b/base/range_iterator.hpp index feb2572314..438fb88516 100644 --- a/base/range_iterator.hpp +++ b/base/range_iterator.hpp @@ -65,28 +65,28 @@ struct RangeWrapper // Use this helper to iterate through 0 to `to'. template -RangeWrapper range(TCounter const to) +RangeWrapper Range(TCounter const to) { return {{}, to}; } // Use this helper to iterate through `from' to `to'. template -RangeWrapper range(TCounter const from, TCounter const to) +RangeWrapper Range(TCounter const from, TCounter const to) { return {from, to}; } // Use this helper to iterate through `from' to 0. template -RangeWrapper reverse_range(TCounter const from) +RangeWrapper ReverseRange(TCounter const from) { return {from, {}}; } // Use this helper to iterate through `from' to `to'. template -RangeWrapper reverse_range(TCounter const from, TCounter const to) +RangeWrapper ReverseRange(TCounter const from, TCounter const to) { return {to, from}; } diff --git a/editor/ui2oh.cpp b/editor/ui2oh.cpp index 0b348e60d6..fcf4c72c41 100644 --- a/editor/ui2oh.cpp +++ b/editor/ui2oh.cpp @@ -71,7 +71,7 @@ bool ConvertOpeningHours(osmoh::OpeningHours const & oh, ui::TimeTableSet & tts) if (oh.IsTwentyFourHours()) return true; - for (auto const & p : my::enumerate(oh.GetRule())) + for (auto const & p : my::Enumerate(oh.GetRule())) { auto const & rulePart = p.item; ui::TimeTable tt; diff --git a/indexer/feature.cpp b/indexer/feature.cpp index 124ef9e91b..78ec41ec1e 100644 --- a/indexer/feature.cpp +++ b/indexer/feature.cpp @@ -57,7 +57,7 @@ FeatureType FeatureType::FromXML(editor::XMLFeature const & xml) // EGeomType - // for (auto const i : range(feature.GetTypesCount())) + // for (auto const i : my::Range(feature.GetTypesCount())) // m_types[i] = // Does the order matter? If so what order should be? // TODO(mgsergio): Only features with single type are currently supported. @@ -65,7 +65,7 @@ FeatureType FeatureType::FromXML(editor::XMLFeature const & xml) feature.m_types[0] = classif().GetTypeByPath({"amenity", "atm"}); feature.m_bTypesParsed = true; - for (auto const i : my::range(1u, static_cast(feature::Metadata::FMD_COUNT))) + for (auto const i : my::Range(1u, static_cast(feature::Metadata::FMD_COUNT))) { auto const type = static_cast(i); auto const attributeName = DebugPrint(type); @@ -102,7 +102,7 @@ editor::XMLFeature FeatureType::ToXML() const // feature.m_params.layer = // feature.m_params.rank = - // for (auto const i : range(feature.GetTypesCount())) + // for (auto const i : my::Range(feature.GetTypesCount())) // m_types[i] = // Does the order matter? If so what order should be? // TODO(mgsergio): Only features with single type are currently supported.