]> OCCT Git - occt-copy.git/commitdiff
0027571: Bad implementation of NCollection_Sequence::Iterator CR27571
authorski <ski@opencascade.com>
Wed, 28 Sep 2016 14:13:47 +0000 (17:13 +0300)
committerski <ski@opencascade.com>
Thu, 29 Sep 2016 10:22:53 +0000 (13:22 +0300)
Reverse iterator after creation points to the first iterated item now (last item of sequence).
Test case was created.

src/NCollection/NCollection_BaseSequence.hxx
src/NCollection/NCollection_Sequence.hxx
src/NCollection/NCollection_StlIterator.hxx
src/QABugs/QABugs_20.cxx
tests/bugs/fclasses/bug27571 [new file with mode: 0644]

index 42f4197b0ed11281f6d634db9c440041c387b877..1be1c86c2c9ba58c6807d2ff5c89e1338647e1a9 100644 (file)
@@ -58,7 +58,7 @@ public:
   {
   public:
     //! Empty constructor
-    Iterator (void) : myCurrent (NULL), myPrevious(NULL) {}
+    Iterator (void) : myCurrent (NULL), myPrevious(NULL), isReversed(Standard_False) {}
 
     //! Constructor with initialisation
     Iterator (const NCollection_BaseSequence& theSeq,
@@ -71,8 +71,9 @@ public:
     void Init (const NCollection_BaseSequence& theSeq,
                const Standard_Boolean isStart = Standard_True)
     {
-      myCurrent  = (isStart ? theSeq.myFirstItem : NULL);
-      myPrevious = (isStart ? NULL : theSeq.myLastItem);
+      myCurrent = (isStart ? theSeq.myFirstItem : theSeq.myLastItem);
+      myPrevious = NULL;
+      isReversed = !isStart;
     }
 
     //! Assignment
@@ -86,13 +87,18 @@ public:
     void Previous()
     {
       myCurrent = myPrevious;
-      if (myCurrent)
-        myPrevious = myCurrent->Previous();
+      if (myCurrent) {
+        if (!isReversed)
+          myPrevious = myCurrent->Previous();
+        else
+          myPrevious = myCurrent->Next();
+      }
     }
       
   protected:
     NCollection_SeqNode* myCurrent;  //!< Pointer to the current node
     NCollection_SeqNode* myPrevious; //!< Pointer to the previous node
+    Standard_Boolean isReversed;
     friend class NCollection_BaseSequence;
   };
 
index f50896cf022fe16459314d26fae9b0636137b509..d8abac79c2e50d484ce5541dee1ee5383b77c32d 100644 (file)
@@ -71,7 +71,10 @@ public:
       if (myCurrent)
       {
         myPrevious = myCurrent;
-        myCurrent = myCurrent->Next();
+        if (isReversed)
+          myCurrent = myCurrent->Previous();
+        else
+          myCurrent = myCurrent->Next();
       }
     }
     //! Constant value access
@@ -93,19 +96,38 @@ public:
   //! Shorthand for a constant iterator type.
   typedef NCollection_StlIterator<std::bidirectional_iterator_tag, Iterator, TheItemType, true> const_iterator;
 
+  //! Shorthand for a reverse iterator type.
+  typedef NCollection_StlIterator<std::reverse_iterator<iterator>, Iterator, TheItemType, false> reverse_iterator;
+
+  //! Shorthand for a constant reverse iterator type.
+  typedef NCollection_StlIterator<std::reverse_iterator<const_iterator>, Iterator, TheItemType, true> const_reverse_iterator;
+
   //! Returns an iterator pointing to the first element in the sequence.
   iterator begin() const { return Iterator (*this, true); }
 
   //! Returns an iterator referring to the past-the-end element in the sequence.
-  iterator end() const { Iterator anIter (*this, false); anIter.Next(); return anIter; }
+  iterator end() const { Iterator anIter (*this, false); anIter.Previous(); return anIter; }
   
   //! Returns a const iterator pointing to the first element in the sequence.
   const_iterator cbegin() const { return Iterator (*this, true); }
 
   //! Returns a const iterator referring to the past-the-end element in the sequence.
-  const_iterator cend() const { Iterator anIter (*this, false); anIter.Next(); return anIter; }
+  const_iterator cend() const { Iterator anIter(*this, false); anIter.Previous(); return anIter; }
 
- public:
+  //! Returns a reverse iterator pointing to the last element in the sequence.
+  reverse_iterator rbegin() const { return Iterator(*this, false); }
+
+  //! Returns a reverse iterator referring to the past-the-end element in the reversed sequence.
+  reverse_iterator rend() const { Iterator anIter(*this, true); anIter.Previous(); return anIter; }
+
+  //! Returns a const reverse iterator pointing to the last element in the sequence.
+  const_reverse_iterator crbegin() const { return Iterator(*this, false); }
+
+  //! Returns a const reverse iterator referring to the past-the-end element in the reversed sequence.
+  const_reverse_iterator crend() const { Iterator anIter(*this, true); anIter.Previous(); return anIter; }
+
+
+public:
   // ---------- PUBLIC METHODS ------------
 
   //! Constructor
index d666270ecda7d9de075420d72c6815958be0b4ac..93e0ed2edd7d244d313c6b84816171523feacf58 100644 (file)
@@ -166,7 +166,9 @@ public: //! @name methods related to bidirectional STL iterator
   NCollection_StlIterator& operator--()
   {
     Standard_STATIC_ASSERT((opencascade::is_same<std::bidirectional_iterator_tag,Category>::value ||
-                            opencascade::is_same<std::random_access_iterator_tag,Category>::value));
+                            opencascade::is_same<std::random_access_iterator_tag,Category>::value ||
+                            opencascade::is_same<std::reverse_iterator<NCollection_StlIterator<std::bidirectional_iterator_tag, BaseIterator, ItemType, false>>, Category>::value ||
+                            opencascade::is_same<std::reverse_iterator<NCollection_StlIterator<std::bidirectional_iterator_tag, BaseIterator, ItemType, true>>, Category>::value));
     myIterator.Previous();
     return *this;
   }
index eae027dba5f49670558d923eec5725eef07247cd..56c7c6900cf39b861a5b9b9f8fcb177d4330bf60 100644 (file)
@@ -2114,6 +2114,76 @@ static Standard_Integer OCC27552(Draw_Interpretor&,
   return 0;
 }
 
+
+#include <NCollection_Sequence.hxx>
+static Standard_Integer OCC27571(Draw_Interpretor& di, Standard_Integer /*nb*/, const char ** /*a*/)
+{
+  // Create NCollection_Sequence<TCollection_AsciiString>
+  NCollection_Sequence<TCollection_AsciiString> aSequence;
+  aSequence.Append("1");
+  aSequence.Append("2");
+  aSequence.Append("3");
+  aSequence.Append("4");
+  aSequence.Append("5");
+
+  // Create iterator
+  NCollection_Sequence<TCollection_AsciiString>::Iterator anIterator(aSequence);
+
+  // Create reverse iterator
+  NCollection_Sequence<TCollection_AsciiString>::Iterator aReverseIterator(aSequence, Standard_False);
+
+  // Display elements using iterator (with More/Next)
+  di << "Iterator (More/Next):";
+  for (; anIterator.More(); anIterator.Next())
+    di << " " << anIterator.Value();
+  anIterator.Previous();
+  di << "\nCurrent (last) element - Iterator: " << anIterator.Value();
+  anIterator.Previous();
+  di << "\nPrevious element - Iterator: " << anIterator.Value();
+
+  // Display elements using reverse iterator (with More/Next)
+  di << "\nReverse iterator (More/Next):";
+  for (; aReverseIterator.More(); aReverseIterator.Next())
+    di << " " << aReverseIterator.Value();
+  aReverseIterator.Previous();
+  di << "\nCurrent (last) element - Reverse iterator: " << aReverseIterator.Value();
+  aReverseIterator.Previous();
+  di << "\nPrevious element - Reverse iterator: " << aReverseIterator.Value();
+
+  // Display elements using iterator (with begin/end)
+  di << "\nIterator (begin/end):";
+  NCollection_Sequence<TCollection_AsciiString>::iterator anIteratorBeginEnd = aSequence.begin();
+  for (; anIteratorBeginEnd != aSequence.end(); ++anIteratorBeginEnd)
+    di << " " << *anIteratorBeginEnd;
+  --anIteratorBeginEnd;
+  di << "\nCurrent (last) element - Iterator (begin/end): " << *anIteratorBeginEnd;
+
+  // Display elements using const_iterator (with cbegin/cend)
+  di << "\nIterator (cbegin/cend) Const:";
+  NCollection_Sequence<TCollection_AsciiString>::const_iterator anIteratorBeginEndConst = aSequence.cbegin();
+  for (; anIteratorBeginEndConst != aSequence.cend(); ++anIteratorBeginEndConst)
+    di << " " << *anIteratorBeginEndConst;
+  --anIteratorBeginEndConst;
+  di << "\nCurrent (last) element - Iterator (cbegin/cend) Const: " << *anIteratorBeginEndConst;
+
+  // Display elements using reverse_iterator (with rbegin/rend)
+  di << "\nReverse iterator (rbegin/rend):";
+  NCollection_Sequence<TCollection_AsciiString>::reverse_iterator aReverseIteratorBeginEnd = aSequence.rbegin();
+  for (; aReverseIteratorBeginEnd != aSequence.rend(); ++aReverseIteratorBeginEnd)
+    di << " " << *aReverseIteratorBeginEnd;
+  --aReverseIteratorBeginEnd;
+  di << "\nCurrent (last) element - Reverse iterator (rbegin/rend): " << *aReverseIteratorBeginEnd;
+
+  // Display elements using const_reverse_iterator (with crbegin/crend)
+  di << "\nReverse iterator (crbegin/crend) Const:";
+  NCollection_Sequence<TCollection_AsciiString>::const_reverse_iterator aReverseIteratorBeginEndConst = aSequence.crbegin();
+  for (; aReverseIteratorBeginEndConst != aSequence.crend(); ++aReverseIteratorBeginEndConst)
+    di << " " << *aReverseIteratorBeginEndConst;
+  --aReverseIteratorBeginEndConst;
+  di << "\nCurrent (last) element - Reverse iterator (crbegin/crend) Const: " << *aReverseIteratorBeginEndConst;
+  return 0;
+}
+
 void QABugs::Commands_20(Draw_Interpretor& theCommands) {
   const char *group = "QABugs";
 
@@ -2132,5 +2202,6 @@ void QABugs::Commands_20(Draw_Interpretor& theCommands) {
   theCommands.Add ("OCC27357", "OCC27357", __FILE__, OCC27357, group);
   theCommands.Add("OCC26270", "OCC26270 shape result", __FILE__, OCC26270, group);
   theCommands.Add ("OCC27552", "OCC27552", __FILE__, OCC27552, group); 
+  theCommands.Add("OCC27571", "OCC27571", __FILE__, OCC27571, group);
   return;
 }
diff --git a/tests/bugs/fclasses/bug27571 b/tests/bugs/fclasses/bug27571
new file mode 100644 (file)
index 0000000..52f0798
--- /dev/null
@@ -0,0 +1,118 @@
+puts "========"
+puts "OCC27571"
+puts "========"
+puts ""
+########################################################
+# Bad implementation of NCollection_Sequence::Iterator
+########################################################
+
+pload QAcommands
+
+set info [OCC27571]
+
+puts ""
+
+set Sequence "1 2 3 4 5"
+set Sequence_Last "5"
+set Sequence_Preult "4"
+set ReversedSequence "5 4 3 2 1"
+set ReversedSequence_Last "1"
+set ReversedSequence_Preult "2"
+
+# Iterator using Init(aSequence, Standard_True) method.
+if { [regexp "Iterator \\(More/Next\\): ${Sequence}" $info] } {
+  puts "OK : Iterator (More/Next)"
+} else {
+  puts "Error : Iterator (More/Next)"
+}
+
+# Method Previous() called after full iteration, current value of Iterator is equal to last element of sequence
+if { [regexp "Current \\(last\\) element - Iterator: ${Sequence_Last}" $info] } {
+  puts "OK : Current (last) element - Iterator"
+} else {
+  puts "Error : Current (last) element - Iterator"
+}
+
+# Method Previous() called once more again, current value of Iterator is equal to penult element of sequence
+if { [regexp "Previous element - Iterator: ${Sequence_Preult}" $info] } {
+  puts "OK : Previous element - Iterator"
+} else {
+  puts "Error : Previous element - Iterator"
+}
+
+# Reverse iterator using Init(aSequence, Standard_False) method.
+if { [regexp "Reverse iterator \\(More/Next\\): ${ReversedSequence}" $info] } {
+  puts "OK : Reverse iterator (More/Next)"
+} else {
+  puts "Error : Reverse iterator (More/Next)"
+}
+
+# Method Previous() called after full iteration, current value of Iterator is equal to last element of sequence
+if { [regexp "Current \\(last\\) element - Reverse iterator: ${ReversedSequence_Last}" $info] } {
+  puts "OK : Current (last) element - Reverse iterator"
+} else {
+  puts "Error : Current (last) element - Reverse iterator"
+}
+
+# Method Previous() called once more again, current value of Iterator is equal to penult element of sequence
+if { [regexp "Previous element - Reverse iterator: ${ReversedSequence_Preult}" $info] } {
+  puts "OK : Previous element - Reverse iterator"
+} else {
+  puts "Error : Previous element - Reverse iterator"
+}
+
+# Iterator using method "begin".
+if { [regexp "Iterator \\(begin/end\\): ${Sequence}" $info] } {
+  puts "OK : Iterator (begin/end)"
+} else {
+  puts "Error : Iterator (begin/end)"
+}
+
+# Check previous element after full iteration, current value of Iterator is equal to last element of sequence
+if { [regexp "Current \\(last\\) element - Iterator \\(begin/end\\): ${Sequence_Last}" $info] } {
+  puts "OK : Current (last) element - Iterator (begin/end)"
+} else {
+  puts "Error : Current (last) element - Iterator (begin/end)"
+}
+
+# Const iterator using method "begin".
+if { [regexp "Iterator \\(cbegin/cend\\) Const: ${Sequence}" $info] } {
+  puts "OK : Iterator (cbegin/cend) Const"
+} else {
+  puts "Error : Iterator (cbegin/cend) Const"
+}
+
+# Check previous element after full iteration, current value of Iterator is equal to last element of sequence
+if { [regexp "Current \\(last\\) element - Iterator \\(cbegin/cend\\) Const: ${Sequence_Last}" $info] } {
+  puts "OK : Current (last) element - Iterator (cbegin/cend) Const"
+} else {
+  puts "Error : Current (last) element - Iterator (cbegin/cend) Const"
+}
+
+# Reverse iterator using method "rbegin".
+if { [regexp "Reverse iterator \\(rbegin/rend\\): ${ReversedSequence}" $info] } {
+  puts "OK : Reverse iterator (rbegin/rend)"
+} else {
+  puts "Error : Reverse iterator (rbegin/rend)"
+}
+
+# Check previous element after full iteration, current value of Iterator is equal to last element of sequence
+if { [regexp "Current \\(last\\) element - Reverse iterator \\(rbegin/rend\\): ${ReversedSequence_Last}" $info] } {
+  puts "OK : Current (last) element - Iterator (rbegin/rend)"
+} else {
+  puts "Error : Current (last) element - Iterator (rbegin/rend)"
+}
+
+# Const reverse iterator using method "crbegin".
+if { [regexp "Reverse iterator \\(crbegin/crend\\) Const: ${ReversedSequence}" $info] } {
+  puts "OK : Reverse iterator (crbegin/crend) Const"
+} else {
+  puts "Error : Reverse iterator (crbegin/crend) Const"
+}
+
+# Check previous element after full iteration, current value of Iterator is equal to last element of sequence
+if { [regexp "Current \\(last\\) element - Reverse iterator \\(crbegin/crend\\) Const: ${ReversedSequence_Last}" $info] } {
+  puts "OK : Current (last) element - Iterator (rbegin/rend) Const"
+} else {
+  puts "Error : Current (last) element - Iterator (rbegin/rend) Const"
+}