From 25646337cd05988a60fc392940747a75a9531f65 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Fri, 2 Jan 2015 13:18:25 +0100 Subject: [PATCH] change list diff language to rely on 'find' instead of 'push' As decided in beb57cde this changeset switches our basic list diff language to work in the style of an insertion sort. Rather than 'pushing back' out-of-order elements, we scan and bring forward missing elements. Later, when passing the original location of the elements fetched this way, a 'skip' verb will help to clean up possible leftowers, so implementation is possible (and indeed acomplished) without shifting any other elements. --- src/lib/diff/list-diff-application.hpp | 40 ++++++++++---------- src/lib/diff/list-diff.hpp | 16 +++++--- tests/library/diff-list-application-test.cpp | 7 ++-- tests/library/diff-list-generation-test.cpp | 7 ++-- 4 files changed, 39 insertions(+), 31 deletions(-) diff --git a/src/lib/diff/list-diff-application.hpp b/src/lib/diff/list-diff-application.hpp index f41e1778c..36bb62d10 100644 --- a/src/lib/diff/list-diff-application.hpp +++ b/src/lib/diff/list-diff-application.hpp @@ -66,6 +66,7 @@ namespace diff{ * @warning behaves only EX_SANE in case of diff application errors, * i.e. only partially modified / rebuilt sequence might be * in the target when diff application is aborted + * @see #ListDiffInterpreter explanation of the verbs */ template class DiffApplicationStrategy> @@ -99,10 +100,11 @@ namespace diff{ } void - __expect_further_elements() + __expect_further_elements (E const& elm) { if (end_of_target()) - throw error::State("Premature end of target sequence; unable to apply diff further." + throw error::State(_Fmt("Premature end of target sequence, still expecting element %s; " + "unable to apply diff further.") % elm , LUMIERA_ERROR_DIFF_CONFLICT); } @@ -111,7 +113,7 @@ namespace diff{ { if (targetPos == orig_.end()) throw error::State(_Fmt("Premature end of sequence; unable to locate " - "element %s as reference point in target.") % elm + "element %s in the remainder of the target.") % elm , LUMIERA_ERROR_DIFF_CONFLICT); } @@ -119,20 +121,20 @@ namespace diff{ /* == Implementation of the diff application primitives == */ void - ins (E elm) + ins (E elm) override { seq_.push_back(elm); } void - del (E elm) + del (E elm) override { __expect_in_target(elm, "remove"); ++pos_; } void - pick (E elm) + pick (E elm) override { __expect_in_target(elm, "pick"); seq_.push_back (move(*pos_)); @@ -140,22 +142,20 @@ namespace diff{ } void - push (E anchor) + skip (E elm) override { - __expect_further_elements(); - E elm(move(*pos_)); // consume current source element + __expect_further_elements (elm); ++pos_; - - // locate the insert position behind the given reference anchor - Iter insertPos = std::find(pos_, orig_.end(), anchor); - __expect_found (anchor, insertPos); - - // inserting the "pushed back" element behind the found position - // this might lead to reallocation and thus invalidate the iterators - auto currIdx = pos_ - orig_.begin(); - orig_.insert (++insertPos, move(elm)); - pos_ = orig_.begin() + currIdx; - } + } // assume the actual content has been moved away by a previous find() + + void + find (E elm) override + { + __expect_further_elements (elm); + Iter found = std::find(pos_, orig_.end(), elm); + __expect_found (elm, found); + seq_.push_back (move(*found)); + } // consume and leave waste, expected to be cleaned-up by skip() later public: diff --git a/src/lib/diff/list-diff.hpp b/src/lib/diff/list-diff.hpp index e0b3068fc..7efc2f0d0 100644 --- a/src/lib/diff/list-diff.hpp +++ b/src/lib/diff/list-diff.hpp @@ -58,10 +58,15 @@ namespace diff{ * included as argument (redundancy). * - \c pick just accepts the \em next element at \em current position into * the resulting altered sequence. The element is given redundantly. - * - \c push effects a re-ordering of the target list contents: it requires - * to \em push the \em next element at \em current processing position - * back further into the list, to be placed at a position \em behind - * the reference element given as argument. + * - \c find effect a re-ordering of the target list contents: it requires + * to \em search for the (next respective single occurrence of the) + * given element further down into the remainder of the list, + * to bring it forward and insert it as the next element. + * - \c skip processing hint, emitted at the position where an element + * previously extracted by a \c find verb happened to sit within + * the old order. This allows an optimising implementation to “fetch” + * a copy and just drop or skip the original, thereby avoiding to + * shift any other elements. */ template class ListDiffInterpreter @@ -72,7 +77,8 @@ namespace diff{ virtual void ins(E e) =0; virtual void del(E e) =0; virtual void pick(E e) =0; - virtual void push(E e) =0; + virtual void find(E e) =0; + virtual void skip(E e) =0; }; template diff --git a/tests/library/diff-list-application-test.cpp b/tests/library/diff-list-application-test.cpp index d0b2668db..48d0dfdec 100644 --- a/tests/library/diff-list-application-test.cpp +++ b/tests/library/diff-list-application-test.cpp @@ -55,7 +55,8 @@ namespace test{ DiffStep_CTOR(ins); DiffStep_CTOR(del); DiffStep_CTOR(pick); - DiffStep_CTOR(push); + DiffStep_CTOR(find); + DiffStep_CTOR(skip); inline DiffSeq @@ -65,11 +66,11 @@ namespace test{ , del(a2) , ins(b1) , pick(a3) - , push(a5) - , pick(a5) + , find(a5) , ins(b2) , ins(b3) , pick(a4) + , skip(a5) , ins(b4) }); diff --git a/tests/library/diff-list-generation-test.cpp b/tests/library/diff-list-generation-test.cpp index b05516604..84888c256 100644 --- a/tests/library/diff-list-generation-test.cpp +++ b/tests/library/diff-list-generation-test.cpp @@ -177,7 +177,8 @@ namespace test{ DiffStep_CTOR(ins); DiffStep_CTOR(del); DiffStep_CTOR(pick); - DiffStep_CTOR(push); + DiffStep_CTOR(find); + DiffStep_CTOR(skip); }//(End)Test fixture @@ -225,11 +226,11 @@ namespace test{ , del(a2) , ins(b1) , pick(a3) - , push(a5) - , pick(a5) + , find(a5) , ins(b2) , ins(b3) , pick(a4) + , skip(a5) , ins(b4) })); }