TreeExplorer / IterSource: document design mismatch (-> Ticket #1125)

IterSource should be refactored to have an iteration control API similar to IterStateWrapper. This would resolve the need to pass that pos-pointer over the abstraction barrier, which is the root cause for all the problems and complexities incurred here
2017-12-09 05:08:24 +01:00 · 2017-12-09 05:08:24 +01:00 · c7e37c29e6
commit c7e37c29e6
parent d56c2295ae
7 changed files with 102 additions and 44 deletions
--- a/src/lib/iter-adapter.hpp
+++ b/src/lib/iter-adapter.hpp
@ -266,7 +266,7 @@ namespace lib {
      const ConRef source() const { return unConst(this)->source_; }
      
      void
-      resetPos (POS otherPos)
+      resetPos (POS otherPos)                                    ////////////////////////////////////////////TICKET #1125 : get rid of this function! it should not be there; rectify IterSource!
        {
          pos_ = otherPos;
          check();
--- a/src/lib/iter-source.hpp
+++ b/src/lib/iter-source.hpp
@ -29,7 +29,11 @@
 ** a simple data source type, without needing to disclose details of
 ** the implementation.
 ** 
- ** \par Standard Adapters
+ ** @todo the design used for the "iteration control API" is misaligned
+ **       with the purpose of this adapter. Rather, it should be shaped
+ **       similar to IterStateWrapper with three control functions     //////////////////////////////////////TICKET #1125
+ ** 
+ ** ## Standard Adapters
 ** As a complement, this header contains a generic implementation
 ** of the IterSource interface by wrapping an existing Lumiera Forward Iterator.
 ** Using this WrappedLumieraIter, the details of this wrapped source iterator
@ -89,7 +93,7 @@ namespace lib {
      typedef TY* Pos;
      typedef shared_ptr<IterSource> DataHandle;
      
-      
+                                                              ///////////////////////////////////////////////TICKET #1125 : this API should use three control functions, similar to IterStateWrapper
      /** iteration start: prepare the first element.
       *  may return NULL in case of empty data source
       */
@ -123,7 +127,7 @@ namespace lib {
      
      /* == Iteration control API for IterAdapter frontend == */
      
-      friend bool
+      friend bool                                                    ////////////////////////////////////////TICKET #1125 : this API should use three control functions, similar to IterStateWrapper
      checkPoint (DataHandle const&, Pos const& pos)
        {
          return bool(pos);
@ -141,7 +145,7 @@ namespace lib {
      
      struct iterator
        : IterAdapter<Pos, DataHandle>
-        {
+        {                                                ////////////////////////////////////////////////////TICKET #1125 : should be build on top of IterStateWrapper rather than IterAdapter!
          using _I = IterAdapter<Pos, DataHandle>;
          using _I::IterAdapter;
          
@ -231,7 +235,7 @@ namespace lib {
      
      IT src_;
      
-      Pos
+      Pos                                                ////////////////////////////////////////////////////TICKET #1125 : this API should use three control functions, similar to IterStateWrapper
      firstResult ()
        {
          if (!src_)
--- a/src/lib/iter-tree-explorer.hpp
+++ b/src/lib/iter-tree-explorer.hpp
@ -737,6 +737,13 @@ namespace lib {
      };
    
    
+    /**
+     * @todo expandChildren() should not return the value pointer.
+     *       This is just a workaround to cope with the design mismatch in IterSource;
+     *       the fact that latter just passes around a pointer into the implementation is
+     *       ugly, dangerous and plain silly.    ////////////////////////////////////////////////////////////TICKET #1125
+     *       Incidentally, this is also the sole reason why this interface is templated with `VAL`
+     */
    template<typename VAL>
    class ChildExpandableSource
      {
@ -755,7 +762,7 @@ namespace lib {
      , public ChildExpandableSource<typename SRC::value_type>
      {
        using Parent = WrappedLumieraIter<SRC>;
-        using Val = typename SRC::value_type;
+        using Val = typename SRC::value_type;                             ///////////////////////////////////TICKET #1125 : get rid of Val
        
      public:
        using Parent::Parent;
@ -764,7 +771,7 @@ namespace lib {
        expandChildren()  override
          {
            Parent::wrappedIter().expandChildren();
-            return Parent::wrappedIter()?  & *Parent::wrappedIter()
+            return Parent::wrappedIter()?  & *Parent::wrappedIter()       ///////////////////////////////////TICKET #1125 : trickery to cope with the misaligned IterSource design
                                        : nullptr;
          }
      };
@ -798,7 +805,7 @@ namespace lib {
          
          auto source = this->source().get();
          VAL* changedResult = dynamic_cast<Expandable*> (source)->expandChildren();
-          this->resetPos (changedResult);
+          this->resetPos (changedResult);                                 ///////////////////////////////////TICKET #1125 : trickery to cope with the misaligned IterSource design
        }
      
      
--- a/tests/core/proc/play/diagnostic-output-slot.hpp
+++ b/tests/core/proc/play/diagnostic-output-slot.hpp
@ -372,15 +372,15 @@ namespace play {
          uint currentFrame_;
          
          
-          virtual Pos
-          firstResult ()
+          virtual Pos                                          //////////////////////////////////////////////TICKET #1125 : this API should use three control functions, similar to IterStateWrapper
+          firstResult ()  override
            {
              REQUIRE (0 == currentFrame_);
              return outSeq_.accessEmittedFrame (currentFrame_);
            }
          
          virtual void
-          nextResult (Pos& pos)
+          nextResult (Pos& pos)  override
            {
              ++currentFrame_;
              pos = outSeq_.accessEmittedFrame(currentFrame_);
--- a/tests/library/iter-source-test.cpp
+++ b/tests/library/iter-source-test.cpp
@ -90,8 +90,8 @@ namespace test{
        string buffer_;
        CStr current_;
        
-        virtual Pos
-        firstResult ()
+        virtual Pos                                    ////////////////////////////////////////////TICKET #1125 : this iteration control API should use three control functions, similar to IterStateWrapper
+        firstResult ()  override
          {
            current_ = buffer_.c_str();
            ENSURE (current_);
@ -99,7 +99,7 @@ namespace test{
          }
        
        virtual void
-        nextResult (Pos& pos)
+        nextResult (Pos& pos)  override
          {
            if (pos && *pos && **pos)
              ++(*pos);
--- a/tests/library/iter-tree-explorer-test.cpp
+++ b/tests/library/iter-tree-explorer-test.cpp
@ -728,30 +728,36 @@ namespace test{
      void
      verify_asIterSource()
        {
-          IterSource<uint>::iterator sequence;
+          IterSource<uint>::iterator sequence;     // note `sequence` is polymorphic
          CHECK (isnil (sequence));
          
          sequence = treeExplore(CountDown{20,10})
                        .filter([](uint i){ return i % 2; })
-                        .asIterSource();
-          
+                        .asIterSource();           // note this terminal builder function
+                                                   // moves the whole pipeline onto the heap
          CHECK (not isnil (sequence));
          CHECK (19 == *sequence);
          
          
+          // use one sequence as source to build another one
          sequence = treeExplore(sequence)
                        .transform([](uint i){ return i*2; })
                        .asIterSource();
          
          CHECK (38 == *sequence);
-          cout << materialise (sequence) <<endl;
+          CHECK ("38-34-30-26-22" == materialise(sequence));
          
+          // WARNING pitfall: `sequence` is a copyable iterator front-end
+          //                  but holds onto the actual pipeline by shared-ptr
+          //                  Thus, even while materialise() creates a copy,
+          //                  the iteration state gets shared....
          CHECK (22 == *sequence);
-          ++sequence;
+          ++sequence;         // ...and even worse, iteration end is only detected after increment
          CHECK (isnil (sequence));
          
          
-          IterExploreSource<uint> exploreIter;
+          // extended API to invoke child expansion opaquely
+          IterExploreSource<char> exploreIter;
          CHECK (isnil (exploreIter));
          
          exploreIter = treeExplore(CountDown{20,10})
@ -759,20 +765,20 @@ namespace test{
                          .transform([](uint i){ return i*2; })
                          .filter([](int i){ return i>25; })
                          .expand([](uint i){ return CountDown{i-10, 20}; })
-//                          .transform([](uint u) -> char { return '@'+u-20; })
-                          .transform([](int u) -> uint { return u; })
+                          .transform([](uint u) -> char { return '@'+u-20; })
                          .asIterSource();
          
          
-//          CHECK (38 == *exploreIter);
+          CHECK ('R' == *exploreIter);   // 38-20 + '@'
          ++exploreIter;
-          exploreIter.expandChildren();
-          cout << *exploreIter << endl;
-          cout << materialise(exploreIter) << endl;
-          cout << *exploreIter << endl;
-#if false /////////////////////////////////////////////////////////////////////////////////////////////////////////////UNIMPLEMENTED :: TICKET #888
-#endif    /////////////////////////////////////////////////////////////////////////////////////////////////////////////UNIMPLEMENTED :: TICKET #888
-        }
+          CHECK ('N' == *exploreIter);   // 34-20 + '@'
+          
+          exploreIter.expandChildren();  // expand consumes the current element (34)
+                                         // and injects the sequence (24...20[ instead
+          CHECK ('D' == *exploreIter);   // 34-10 == 24 and 'D' ==  24-20 + '@'
+          
+          CHECK ("D-C-B-A-J-F" == materialise(exploreIter));
+        }                                // note how the remainder of the original sequence is picked up with 'J'...
      
      
      /** @test use a preconfigured exploration scheme to expand depth-first until exhaustion
--- a/wiki/thinkPad.ichthyo.mm
+++ b/wiki/thinkPad.ichthyo.mm
@ -5729,7 +5729,7 @@
 </node>
 <node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1512363286920" ID="ID_1420123769" MODIFIED="1512363305422" TEXT="Zusammenspiel">
 <icon BUILTIN="flag-yellow"/>
-<node COLOR="#338800" CREATED="1512363297486" FOLDED="true" ID="ID_531127457" MODIFIED="1512521973765" TEXT="re-evaluate">
+<node COLOR="#338800" CREATED="1512363297486" FOLDED="true" ID="ID_531127457" MODIFIED="1512794302599" TEXT="re-evaluate">
 <linktarget COLOR="#5a92a2" DESTINATION="ID_531127457" ENDARROW="Default" ENDINCLINATION="385;167;" ID="Arrow_ID_556443427" SOURCE="ID_579206895" STARTARROW="None" STARTINCLINATION="844;55;"/>
 <icon BUILTIN="button_ok"/>
 <icon BUILTIN="messagebox_warning"/>
@ -5942,7 +5942,7 @@
 <node CREATED="1512516543648" ID="ID_1021495966" MODIFIED="1512516558578" TEXT="transformer mu&#xdf; daraufhin seinen Cache wegwerfen"/>
 <node CREATED="1512516567076" ID="ID_647647031" MODIFIED="1512516587342" TEXT="filter mu&#xdf; daraufhin erneut filter-pull ausf&#xfc;hren"/>
 </node>
-<node CREATED="1512516613614" ID="ID_1415224184" MODIFIED="1512516850054">
+<node CREATED="1512516613614" ID="ID_1415224184" MODIFIED="1512794145729">
 <richcontent TYPE="NODE"><html>
  <head>
    
@ -5952,7 +5952,7 @@
      Konzept funktioniert nicht
    </p>
    <p>
-      iaus processing-Function
+      aus processing-Function
    </p>
    <p>
      innen heraus
@ -6015,6 +6015,36 @@
 <icon BUILTIN="ksmiletris"/>
 </node>
 </node>
+<node CREATED="1512794157617" ID="ID_1639627950" MODIFIED="1512794164448" TEXT="Problem mit IterSource">
+<icon BUILTIN="messagebox_warning"/>
+<node CREATED="1512794166904" ID="ID_614780435" LINK="http://issues.lumiera.org/ticket/1125" MODIFIED="1512794180867" TEXT="Design-Mismatch">
+<icon BUILTIN="info"/>
+</node>
+<node CREATED="1512794183629" ID="ID_783686672" MODIFIED="1512794297194" TEXT="inzwischen ein Workaround">
+<richcontent TYPE="NOTE"><html>
+  <head>
+    
+  </head>
+  <body>
+    <ul>
+      <li>
+        asIterSource() verwendet ein eigenes Interface, um diesen call an die Implementierung durchzureichen
+      </li>
+      <li>
+        in dieses Interface habe ich nun einen R&#252;ckgabewert eingebaut
+      </li>
+      <li>
+        damit kann ich das IterSource-Front-End refreshen
+      </li>
+      <li>
+        trotzdem h&#228;sslich...
+      </li>
+    </ul>
+  </body>
+</html>
+</richcontent>
+</node>
+</node>
 </node>
 </node>
 </node>
@ -6145,8 +6175,8 @@
 </node>
 </node>
 </node>
-<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1512621185490" ID="ID_1611878074" MODIFIED="1512787526602" TEXT="Probleme">
-<icon BUILTIN="flag-yellow"/>
+<node COLOR="#338800" CREATED="1512621185490" FOLDED="true" ID="ID_1611878074" MODIFIED="1512794118863" TEXT="Probleme">
+<icon BUILTIN="button_ok"/>
 <node CREATED="1512621202688" ID="ID_57740660" MODIFIED="1512706113620" TEXT="durchreichen der Basis-Konstruktoren">
 <icon BUILTIN="info"/>
 <node CREATED="1512706139167" ID="ID_1731306892" MODIFIED="1512706156440" TEXT="Basis-Copy-Konstruktoren werden nicht geerbt">
@ -6269,7 +6299,7 @@
 <icon BUILTIN="idea"/>
 </node>
 </node>
-<node CREATED="1512621222901" ID="ID_701320194" MODIFIED="1512706102580" TEXT="bad function call">
+<node CREATED="1512621222901" FOLDED="true" ID="ID_701320194" MODIFIED="1512794109999" TEXT="bad function call">
 <icon BUILTIN="button_ok"/>
 <node CREATED="1512621654283" ID="ID_1975194377" MODIFIED="1512621667489" TEXT="passiert im FIlter-Iterator-Test">
 <icon BUILTIN="messagebox_warning"/>
@ -6288,8 +6318,8 @@
 <node CREATED="1512706069104" ID="ID_596682451" MODIFIED="1512706085953" TEXT="und deshalb ist nicht aufgefallen, da&#xdf; das Objekt schon tot war"/>
 </node>
 </node>
-<node BACKGROUND_COLOR="#fdfdcf" COLOR="#ff0000" CREATED="1512787530248" ID="ID_1415286168" MODIFIED="1512787540703" TEXT="IterSource design mismatch">
-<icon BUILTIN="flag-pink"/>
+<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1512787530248" FOLDED="true" ID="ID_1415286168" MODIFIED="1512794112672" TEXT="IterSource design mismatch">
+<icon BUILTIN="hourglass"/>
 <node CREATED="1512788814147" ID="ID_499062281" MODIFIED="1512788850106" TEXT="versteckt gecachete Auswertung im Pos-Pointer">
 <icon BUILTIN="messagebox_warning"/>
 </node>
@ -6309,9 +6339,20 @@
 <icon BUILTIN="yes"/>
 <node CREATED="1512789836161" ID="ID_1758704874" MODIFIED="1512789845955" TEXT="Design von IterSource ist nicht in Ordnung"/>
 <node CREATED="1512789847183" ID="ID_1699957562" MODIFIED="1512789865193" TEXT="es sollte sich am &quot;state core&quot;-Schema orientieren"/>
-<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1512789870516" ID="ID_1318870340" MODIFIED="1512790017742" TEXT="Ticket machen">
+<node CREATED="1512789870516" ID="ID_1318870340" LINK="http://issues.lumiera.org/ticket/1125" MODIFIED="1512794035043">
+<richcontent TYPE="NODE"><html>
+  <head>
+    
+  </head>
+  <body>
+    <p>
+      Ticket machen: <font color="#b90736">#1125</font>
+    </p>
+  </body>
+</html>
+</richcontent>
 <linktarget COLOR="#7f5f85" DESTINATION="ID_1318870340" ENDARROW="Default" ENDINCLINATION="867;50;" ID="Arrow_ID_1918593298" SOURCE="ID_1066396845" STARTARROW="None" STARTINCLINATION="483;0;"/>
-<icon BUILTIN="flag-yellow"/>
+<icon BUILTIN="button_ok"/>
 </node>
 </node>
 </node>
@ -6655,12 +6696,12 @@
 <node CREATED="1512787549870" ID="ID_1969077394" MODIFIED="1512789984536" TEXT="wieder mal kein refresh nach expandChildren()">
 <icon BUILTIN="info"/>
 </node>
-<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1512788719640" ID="ID_1066396845" MODIFIED="1512790017742" TEXT="TODO: Design-Mismatch -&gt; Ticket">
+<node COLOR="#338800" CREATED="1512788719640" ID="ID_1066396845" LINK="http://issues.lumiera.org/ticket/1125" MODIFIED="1512794080241" TEXT="TODO: Design-Mismatch -&gt; Ticket">
 <arrowlink COLOR="#7f5f85" DESTINATION="ID_1318870340" ENDARROW="Default" ENDINCLINATION="867;50;" ID="Arrow_ID_1918593298" STARTARROW="None" STARTINCLINATION="483;0;"/>
-<icon BUILTIN="flag-yellow"/>
+<icon BUILTIN="button_ok"/>
 </node>
 </node>
-<node CREATED="1512621383815" ID="ID_1705587551" MODIFIED="1512789963438" TEXT="Problem: shallow copy">
+<node CREATED="1512621383815" FOLDED="true" ID="ID_1705587551" MODIFIED="1512794312525" TEXT="Problem: shallow copy">
 <richcontent TYPE="NOTE"><html>
  <head>