Stream Replacers chain has been simplified. Ownership of the chain ha…

…ve been reworked (zproksi#17)

* Stream Replacers chain has been simplified. Ownership of the chain have been reworked
* method SetNextReplacer got the name SetLastReplacer

---------

Co-authored-by: Alexey Zaytsev <Zaytsev.Lesha@gmail.com>

srcbpatch/actionscollection.cpp

@@ -273,25 +273,24 @@ void ActionsCollection::OnJsonArrayParsed(TJSONObject* const pJson)
 
 void ActionsCollection::DoReplacements(const char toProcess, const bool aEod) const
 {
-    replacersChain_.front()->DoReplacements(toProcess, aEod);
+    replacersChain_->DoReplacements(toProcess, aEod);
 }
 
-void ActionsCollection::SetNextReplacer(StreamReplacer* const pNext)
+
+void ActionsCollection::SetLastReplacer(std::unique_ptr<StreamReplacer>&& pNext)
 {
     if (!pNext)
     {
         throw std::logic_error("Action Collection have not initilized properly. Contact with maintainer.");
     }
-    replacersChain_.back()->SetNextReplacer(pNext);
+    replacersChain_->SetLastReplacer(std::move(pNext)); // now full chain in replacer
 }
 
-
 void ActionsCollection::ReportError(const char* const message)
 {
     throw std::runtime_error(message);
 }
 
-
 void ActionsCollection::ReportDuplicateNameError(const std::string_view& aname)
 {
     std::stringstream ss;
@@ -439,9 +438,7 @@ void ActionsCollection::CreateChainOfReplacers()
         ReportError("Nothing to replace in todo array of Actions file");
     }
 
-    // last replacer in chain, need to be established in our SetNextReplacer
-    StreamReplacer* pNext = nullptr;
-    for(auto rit = replaces_.rbegin(); rit != replaces_.rend(); ++rit) // from the end
+    for (auto rit = replaces_.crbegin(); rit != replaces_.crend(); ++rit) // from the end
     {
         const VectorStringviewPairs& vPairs = *rit;
         const size_t nReplaces = vPairs.size();
@@ -473,12 +470,10 @@ void ActionsCollection::CreateChainOfReplacers()
         }
         // create replacer
         std::unique_ptr<StreamReplacer> replacer = StreamReplacer::CreateReplacer(sourceTargetPairs);
-
-        // set chain
-        replacer->SetNextReplacer(pNext);
-        pNext = replacer.get();// now we are next to establish
-        replacersChain_.emplace_front(std::move(replacer)); // save in front - we will use front
-
+        // `replacer` needs to hold tail of the chain
+        // replacersChain_ contains the tail of chain
+        replacer->SetLastReplacer(std::move(replacersChain_)); // now full chain is in replacer
+        replacersChain_ = std::move(replacer); // now full chain is in place
     } // for(auto rit = replaces_.rbegin(); rit != replaces_.rend(); ++rit)
 
     // everything has been created. free some memory

srcbpatch/actionscollection.h

@@ -59,11 +59,10 @@ class ActionsCollection final: public TJsonCallBack, public StreamReplacer
     void DoReplacements(const char toProcess, const bool aEod) const override;
 
     /// <summary>
-    ///   callback from StreamReplacer
+    ///   Set next replacer in chain of replacers
     /// </summary>
-    /// <param name="toProcess">replacer to call next</param>
-    void SetNextReplacer(StreamReplacer* const pNext) override;
-
+    /// <param name="pNext">replacer to call next</param>
+    virtual void SetLastReplacer(std::unique_ptr<StreamReplacer>&& pNext) override;
 
 protected:
     /// <summary>
@@ -136,7 +135,7 @@ class ActionsCollection final: public TJsonCallBack, public StreamReplacer
     /// <summary>
     ///  here we are holding chain of the replacers
     /// </summary>
-    std::list<std::unique_ptr<StreamReplacer>> replacersChain_;
+    std::unique_ptr<StreamReplacer> replacersChain_;
 }; // class ActionsCollection
 
 

srcbpatch/processing.cpp

@@ -61,8 +61,7 @@ void DoReadReplaceWrite(unique_ptr<ActionsCollection>& todo, Reader* const pRead
 {
     using namespace std;
     // setup chain to write the data
-    auto lastReplacer = StreamReplacer::ReplacerLastInChain(pWriter);
-    todo->SetNextReplacer(lastReplacer.get());
+    todo->SetLastReplacer(StreamReplacer::ReplacerLastInChain(pWriter));
 
     // hold vector where we are reading data.
     // no new allocations

srcbpatch/streamreplacer.cpp

@@ -17,7 +17,7 @@ class WriterReplacer final : public StreamReplacer
     WriterReplacer(Writer* const pWriter): pWriter_(pWriter){};
 
     virtual void DoReplacements(const char toProcess, const bool aEod) const override;
-    virtual void SetNextReplacer(StreamReplacer* const pNext) override;
+    virtual void SetLastReplacer(std::unique_ptr<StreamReplacer>&& pNext) override;
 protected:
     Writer* const pWriter_;
 };
@@ -29,7 +29,7 @@ void WriterReplacer::DoReplacements(const char toProcess, const bool aEod) const
 }
 
 
-void WriterReplacer::SetNextReplacer(StreamReplacer* const)
+void WriterReplacer::SetLastReplacer(std::unique_ptr<StreamReplacer>&&)
 {
     throw logic_error("Writer Replacer should be unchangeable. Contact with maintainer.");
 }
@@ -49,13 +49,21 @@ class ReplacerWithNext: public StreamReplacer
 {
 protected:
     /// to pass processing further
-    StreamReplacer* pNext_ = nullptr;
+    std::unique_ptr<StreamReplacer> pNext_;
 
 public:
-    void SetNextReplacer(StreamReplacer* const pNext) override
+    void SetLastReplacer(std::unique_ptr<StreamReplacer>&& pNext) override
     {
-        pNext_ = pNext;
+        if (pNext_)
+        {
+            pNext_->SetLastReplacer(std::move(pNext));
+        }
+        else
+        {
+            pNext_ = std::move(pNext);
+        }
     }
+
 };
 
 

srcbpatch/streamreplacer.h

@@ -40,10 +40,10 @@ struct StreamReplacer
 
 
     /// <summary>
-    ///   callback from StreamReplacer
+    ///   Set last replacer in chain of replacers. (Add it as the last in the chain, actually)
     /// </summary>
-    /// <param name="toProcess">replacer to call next</param>
-    virtual void SetNextReplacer(StreamReplacer* const pNext) = 0;
+    /// <param name="pNext">replacer to call next</param>
+    virtual void SetLastReplacer(std::unique_ptr<StreamReplacer>&& pNext) = 0;
 
 
     virtual ~StreamReplacer() = default;
@@ -54,6 +54,7 @@ struct StreamReplacer
 
 /// <summary>
 ///    creates a StreamReplacer for writing the lexemes to file
+/// Adding another StreamReplacer after adding this one leads to exception
 /// </summary>
 /// <param name="pWriter">writer interface</param>
 /// <returns>unique_ptr with object which should be last in chain of processing</returns>

testbpatch/test.cpp

@@ -689,8 +689,7 @@ TEST(ACollection, ActionsProcessing)
         using namespace bpatch;
         ActionsCollection ac(move(vec)); // processor
         TestWriter tw; // here we accumulating data
-        auto lastReplacer = StreamReplacer::ReplacerLastInChain(&tw);
-        ac.SetNextReplacer(lastReplacer.get()); // set write point
+        ac.SetLastReplacer(StreamReplacer::ReplacerLastInChain(&tw)); // set write point
 
 
         std::ranges::for_each(tst.testData, [&ac](const char c) {ac.DoReplacements(c, false); });
@@ -958,8 +957,7 @@ TEST(ACollection, MultipleUsageOfProcessing)
         using namespace bpatch;
         ActionsCollection ac(move(vec)); // processor
         TestWriter tw; // here we accumulating data
-        auto lastReplacer = StreamReplacer::ReplacerLastInChain(&tw);
-        ac.SetNextReplacer(lastReplacer.get()); // set write point
+        ac.SetLastReplacer(StreamReplacer::ReplacerLastInChain(&tw)); // set write point
 
 
         std::ranges::for_each(tst.testData, [&ac](const char c) {ac.DoReplacements(c, false); });