ChainedIterator#toString doesn't exhaust iterator itself
authorMikhail Golubev <mikhail.golubev@jetbrains.com>
Fri, 25 Sep 2015 11:50:37 +0000 (14:50 +0300)
committerMikhail Golubev <mikhail.golubev@jetbrains.com>
Fri, 25 Sep 2015 12:38:34 +0000 (15:38 +0300)
Implementation of toString() with side-effects is horrible
(especially for future debugging).

python/src/com/jetbrains/python/toolbox/ChainIterationMixin.java
python/src/com/jetbrains/python/toolbox/ChainedListBase.java
python/testSrc/com/jetbrains/python/toolbox/IteratorsTest.java

index 75141d62db00c8f95499fb78bc1f320057d472bc..1387a40f04c8fabf7a8e161110f0423b2bdc9f3d 100644 (file)
@@ -40,22 +40,21 @@ abstract class ChainIterationMixin<T, TPayload> {
   // returns either null or a non-exhausted iterator.
   @Nullable
   public Iterator<T> getCurrent() {
-    while ((myCurrent == null || !myCurrent.hasNext()) && myLink.hasPayload()) { // fix myCurrent
+    while ((myCurrent == null || !myCurrent.hasNext()) && (myLink != null && myLink.myPayload != null)) { // fix myCurrent
       if (myCurrent == null) {
         myCurrent = toIterator(myLink.myPayload);
         assert myCurrent != null;
       }
       else {
-        myLink.moveOn();
+        myLink= myLink.myNext;
         myCurrent = null;
       }
     }
     return myCurrent;
-  }
+  } 
 
   public boolean hasNext() {
-    Iterator<T> current = getCurrent();
-    return (current != null);
+    return getCurrent() != null;
   }
 
   public T next() {
index 23997a2ad7c183ef1ed4b54958ff768243428e59..d1e5651ea9f3591f156f78711cec34d7d4311c24 100644 (file)
@@ -43,7 +43,9 @@ public /*abstract */class ChainedListBase<TPayload> {
    * @return
    */
   protected ChainedListBase<TPayload> add(TPayload another) {
-    if (myPayload == null) myPayload = another;
+    if (myPayload == null) {
+      myPayload = another;
+    }
     else {
       ChainedListBase<TPayload> farthest = this;
       while (farthest.myNext != null) farthest = farthest.myNext;
@@ -51,17 +53,4 @@ public /*abstract */class ChainedListBase<TPayload> {
     }
     return this;
   }
-
-  // become to our next
-  public void moveOn() {
-    if (myNext != null) {
-      myPayload = myNext.myPayload;
-      myNext = myNext.myNext;
-    }
-    else myPayload = null; // position 'after the end'
-  }
-
-  public boolean hasPayload() {
-    return myPayload != null;
-  }
 }
index 3db0e0316eabd60c574e6a5d148d5885f220773f..918615eb38e7ebaea403ccae7a9eacc7195cd833 100644 (file)
@@ -190,5 +190,10 @@ public class IteratorsTest extends TestCase {
     assertEquals(all.size(), count);
   }
 
-
+  public void testToStringDoesntExhaustIterator() {
+    final ChainIterable<String> initial = new ChainIterable<String>();
+    initial.addItem("foo");
+    assertEquals("foo", initial.toString());;
+    assertEquals("foo", initial.toString());
+  }
 }