Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #593 Fix the arithmetic operators in the iterator and reverse iterator #595

Merged
merged 14 commits into from
Jun 10, 2017
Merged

Issue #593 Fix the arithmetic operators in the iterator and reverse iterator #595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
ed62129
Override n + iterator operator in the iterator
HenryRLee May 27, 2017
adc73d6
Fix the arithmetic mistakes in reverse iterator
HenryRLee May 29, 2017
a3bf013
Fix some more reverse iterator arithmetic test codes
HenryRLee May 29, 2017
3aef1a5
Change the definition of the operator override of reverse iterator to…
HenryRLee May 29, 2017
e42db27
Fix the indentation from the latest commit
HenryRLee May 29, 2017
5b245da
Comment out the exception message check in operator[] of reverse iter…
HenryRLee May 29, 2017
0c2ed00
Add missing test cases for n + it arithmetic in iterators
HenryRLee May 29, 2017
daea213
Use static cast in the implementation of the operators of reverse ite…
HenryRLee May 29, 2017
c4ab8f8
Change iterator category to random access
HenryRLee May 30, 2017
fb91aa8
Merge branch 'develop' into iterator_arithmetic
HenryRLee Jun 5, 2017
c98169d
Revert "Change iterator category to random access"
HenryRLee Jun 5, 2017
0f065ed
Revert a change in reverse iterator to pass the test cases
HenryRLee Jun 7, 2017
6661ec7
Merge branch 'develop' into iterator_arithmetic
HenryRLee Jun 7, 2017
e12c2ee
Merge branch 'develop' into iterator_arithmetic
HenryRLee Jun 9, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 21 additions & 17 deletions src/json.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8503,18 +8503,29 @@ class basic_json
@brief add to iterator
@pre The iterator is initialized; i.e. `m_object != nullptr`.
*/
iter_impl operator+(difference_type i)
iter_impl operator+(difference_type i) const
{
auto result = *this;
result += i;
return result;
}

/*!
@brief addition of distance and iterator
@pre The iterator is initialized; i.e. `m_object != nullptr`.
*/
friend iter_impl operator+(difference_type i, const iter_impl& it)
{
auto result = it;
result += i;
return result;
}

/*!
@brief subtract from iterator
@pre The iterator is initialized; i.e. `m_object != nullptr`.
*/
iter_impl operator-(difference_type i)
iter_impl operator-(difference_type i) const
{
auto result = *this;
result -= i;
Expand Down Expand Up @@ -8656,56 +8667,49 @@ class basic_json
/// post-increment (it++)
json_reverse_iterator operator++(int)
{
return base_iterator::operator++(1);
return static_cast<json_reverse_iterator>(base_iterator::operator++(1));
}

/// pre-increment (++it)
json_reverse_iterator& operator++()
{
base_iterator::operator++();
return *this;
return static_cast<json_reverse_iterator&>(base_iterator::operator++());
}

/// post-decrement (it--)
json_reverse_iterator operator--(int)
{
return base_iterator::operator--(1);
return static_cast<json_reverse_iterator>(base_iterator::operator--(1));
}

/// pre-decrement (--it)
json_reverse_iterator& operator--()
{
base_iterator::operator--();
return *this;
return static_cast<json_reverse_iterator&>(base_iterator::operator--());
}

/// add to iterator
json_reverse_iterator& operator+=(difference_type i)
{
base_iterator::operator+=(i);
return *this;
return static_cast<json_reverse_iterator&>(base_iterator::operator+=(i));
}

/// add to iterator
json_reverse_iterator operator+(difference_type i) const
{
auto result = *this;
result += i;
return result;
return static_cast<json_reverse_iterator>(base_iterator::operator+(i));
}

/// subtract from iterator
json_reverse_iterator operator-(difference_type i) const
{
auto result = *this;
result -= i;
return result;
return static_cast<json_reverse_iterator>(base_iterator::operator-(i));
}

/// return difference
difference_type operator-(const json_reverse_iterator& other) const
{
return this->base() - other.base();
return base_iterator(*this) - base_iterator(other);
}

/// access to successor
Expand Down
44 changes: 38 additions & 6 deletions test/src/unit-iterators2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,16 @@ TEST_CASE("iterators 2")
CHECK_THROWS_AS(it + 1, json::invalid_iterator);
CHECK_THROWS_WITH(it + 1, "[json.exception.invalid_iterator.209] cannot use offsets with object iterators");
}
{
auto it = j_object.begin();
CHECK_THROWS_AS(1 + it, json::invalid_iterator);
CHECK_THROWS_WITH(1 + it, "[json.exception.invalid_iterator.209] cannot use offsets with object iterators");
}
{
auto it = j_object.cbegin();
CHECK_THROWS_AS(1 + it, json::invalid_iterator);
CHECK_THROWS_WITH(1 + it, "[json.exception.invalid_iterator.209] cannot use offsets with object iterators");
}
{
auto it = j_object.begin();
CHECK_THROWS_AS(it -= 1, json::invalid_iterator);
Expand Down Expand Up @@ -307,6 +317,7 @@ TEST_CASE("iterators 2")
auto it = j_array.begin();
it += 3;
CHECK((j_array.begin() + 3) == it);
CHECK(json::iterator(3 + j_array.begin()) == it);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason the addition is written the other way?

Copy link
Contributor Author

@HenryRLee HenryRLee Jun 5, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a required operation in a random accessible iterator. Please check: http://en.cppreference.com/w/cpp/concept/RandomAccessIterator

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh stupid me... I didn't see you added a line - I thought you changed the line to this order...

CHECK((it - 3) == j_array.begin());
CHECK((it - j_array.begin()) == 3);
CHECK(*it == json(4));
Expand All @@ -317,6 +328,7 @@ TEST_CASE("iterators 2")
auto it = j_array.cbegin();
it += 3;
CHECK((j_array.cbegin() + 3) == it);
CHECK(json::const_iterator(3 + j_array.cbegin()) == it);
CHECK((it - 3) == j_array.cbegin());
CHECK((it - j_array.cbegin()) == 3);
CHECK(*it == json(4));
Expand All @@ -331,6 +343,7 @@ TEST_CASE("iterators 2")
auto it = j_null.begin();
it += 3;
CHECK((j_null.begin() + 3) == it);
CHECK(json::iterator(3 + j_null.begin()) == it);
CHECK((it - 3) == j_null.begin());
CHECK((it - j_null.begin()) == 3);
CHECK(it != j_null.end());
Expand All @@ -341,6 +354,7 @@ TEST_CASE("iterators 2")
auto it = j_null.cbegin();
it += 3;
CHECK((j_null.cbegin() + 3) == it);
CHECK(json::const_iterator(3 + j_null.cbegin()) == it);
CHECK((it - 3) == j_null.cbegin());
CHECK((it - j_null.cbegin()) == 3);
CHECK(it != j_null.cend());
Expand All @@ -355,6 +369,7 @@ TEST_CASE("iterators 2")
auto it = j_value.begin();
it += 3;
CHECK((j_value.begin() + 3) == it);
CHECK(json::iterator(3 + j_value.begin()) == it);
CHECK((it - 3) == j_value.begin());
CHECK((it - j_value.begin()) == 3);
CHECK(it != j_value.end());
Expand All @@ -365,6 +380,7 @@ TEST_CASE("iterators 2")
auto it = j_value.cbegin();
it += 3;
CHECK((j_value.cbegin() + 3) == it);
CHECK(json::const_iterator(3 + j_value.cbegin()) == it);
CHECK((it - 3) == j_value.cbegin());
CHECK((it - j_value.cbegin()) == 3);
CHECK(it != j_value.cend());
Expand Down Expand Up @@ -688,6 +704,16 @@ TEST_CASE("iterators 2")
CHECK_THROWS_AS(it + 1, json::invalid_iterator);
CHECK_THROWS_WITH(it + 1, "[json.exception.invalid_iterator.209] cannot use offsets with object iterators");
}
{
auto it = j_object.rbegin();
CHECK_THROWS_AS(1 + it, json::invalid_iterator);
CHECK_THROWS_WITH(1 + it, "[json.exception.invalid_iterator.209] cannot use offsets with object iterators");
}
{
auto it = j_object.crbegin();
CHECK_THROWS_AS(1 + it, json::invalid_iterator);
CHECK_THROWS_WITH(1 + it, "[json.exception.invalid_iterator.209] cannot use offsets with object iterators");
}
{
auto it = j_object.rbegin();
CHECK_THROWS_AS(it -= 1, json::invalid_iterator);
Expand Down Expand Up @@ -726,8 +752,9 @@ TEST_CASE("iterators 2")
auto it = j_array.rbegin();
it += 3;
CHECK((j_array.rbegin() + 3) == it);
CHECK(json::reverse_iterator(3 + j_array.rbegin()) == it);
CHECK((it - 3) == j_array.rbegin());
CHECK((j_array.rbegin() - it) == 3);
CHECK((it - j_array.rbegin()) == 3);
CHECK(*it == json(3));
it -= 2;
CHECK(*it == json(5));
Expand All @@ -736,8 +763,9 @@ TEST_CASE("iterators 2")
auto it = j_array.crbegin();
it += 3;
CHECK((j_array.crbegin() + 3) == it);
CHECK(json::const_reverse_iterator(3 + j_array.crbegin()) == it);
CHECK((it - 3) == j_array.crbegin());
CHECK((j_array.crbegin() - it) == 3);
CHECK((it - j_array.crbegin()) == 3);
CHECK(*it == json(3));
it -= 2;
CHECK(*it == json(5));
Expand All @@ -750,8 +778,9 @@ TEST_CASE("iterators 2")
auto it = j_null.rbegin();
it += 3;
CHECK((j_null.rbegin() + 3) == it);
CHECK(json::reverse_iterator(3 + j_null.rbegin()) == it);
CHECK((it - 3) == j_null.rbegin());
CHECK((j_null.rbegin() - it) == 3);
CHECK((it - j_null.rbegin()) == 3);
CHECK(it != j_null.rend());
it -= 3;
CHECK(it == j_null.rend());
Expand All @@ -760,8 +789,9 @@ TEST_CASE("iterators 2")
auto it = j_null.crbegin();
it += 3;
CHECK((j_null.crbegin() + 3) == it);
CHECK(json::const_reverse_iterator(3 + j_null.crbegin()) == it);
CHECK((it - 3) == j_null.crbegin());
CHECK((j_null.crbegin() - it) == 3);
CHECK((it - j_null.crbegin()) == 3);
CHECK(it != j_null.crend());
it -= 3;
CHECK(it == j_null.crend());
Expand All @@ -774,8 +804,9 @@ TEST_CASE("iterators 2")
auto it = j_value.rbegin();
it += 3;
CHECK((j_value.rbegin() + 3) == it);
CHECK(json::reverse_iterator(3 + j_value.rbegin()) == it);
CHECK((it - 3) == j_value.rbegin());
CHECK((j_value.rbegin() - it) == 3);
CHECK((it - j_value.rbegin()) == 3);
CHECK(it != j_value.rend());
it -= 3;
CHECK(*it == json(42));
Expand All @@ -784,8 +815,9 @@ TEST_CASE("iterators 2")
auto it = j_value.crbegin();
it += 3;
CHECK((j_value.crbegin() + 3) == it);
CHECK(json::const_reverse_iterator(3 + j_value.crbegin()) == it);
CHECK((it - 3) == j_value.crbegin());
CHECK((j_value.crbegin() - it) == 3);
CHECK((it - j_value.crbegin()) == 3);
CHECK(it != j_value.crend());
it -= 3;
CHECK(*it == json(42));
Expand Down