corrections to chapter 18 from email

Author: justinmeiners

18_binary_insertion_sort.md

@@ -10,7 +10,7 @@ which would indicate what mood I'm in.
 This is a very great song by [Franz Schubert][schubert] but it also
 perfectly reflects what will go on with the course in how I feel.
 The song is called ["The Organ Grinder"][winter-journey] (Der Leiermann).
-The singer is [Dietrich fischer-dieskau][dietrich] maybe the greatest
+The singer is [Dietrich Fischer-Dieskau][dietrich] maybe the greatest
 leader, or art song singer of the last 50, 60, or 70 years.
 He started singing in the late forties.
 Let us spend a couple of minutes and listen to it... ([Video here][organ-grinder])
@@ -263,25 +263,23 @@ That's the invariant on which we rely.
 
 ### Rotate for bidirectional iterators
 
-Once we find where it goes, how do we make room for it?
+Once we find where it goes (the element to insert), how do we make room for it?
 We "rotate" to the right by one.
-If it was a bidirectional iterator there is a beautiful algorithm.
-Copy is the wrong thing, because it will overwrite everything
-with the same value.
-What we want is copying from the back.
+If it is a bidirectional iterator there is a beautiful algorithm, copying from the back.
 The algorithm is called: [`std::copy_backward`][cpp-copy-back].
 
     template <typename I>
     // I is BidirectionalIterator
     void rotate_right_by_one(I first, I last, std::bidirectional_iterator_tag) {
       typedef typename std::iterator_traits<I>::value_type T;
-      T butlast = last;
+      I butlast = last;
       --butlast;
       T x = *butlast; 
       std::copy_backward(first, butlast, last);
       *first = x;
     }
 
+Note that forward copy is the wrong thing, because it will overwrite everything with the same value (namely the first value).
 
 ### Rotate for forward iterators
 
@@ -349,16 +347,16 @@ It might not be the fastest, but it is going to be much more elegant.
     void rotate_right_by_one(I first, I last, std::forward_iterator_tag) {
         if (first == last) return;
         I current = first;
-        while (++current != last) std::swap(first, current);
+        while (++current != last) std::swap(*first, *current);
     }
 
 Let's write a dispatch for both versions,
 it will compile to no code[^concepts-in-standard-soon].
 
     template <typename I>
     inline
-    void rotate_right_by_one(I first, I butlast, I last) {
-      rotate_right_by_one(first, butlast, last, typename std::iterator_traits<I>::iterator_category());
+    void rotate_right_by_one(I first, I last) {
+      rotate_right_by_one(first, last, typename std::iterator_traits<I>::iterator_category());
     }
 
 

code/binary_insertion_sort.h

@@ -11,7 +11,7 @@ template <typename I>
 void rotate_right_by_one(I first, I last, std::forward_iterator_tag) {
   if (first == last) return;
   I current = first; 
-  while (++current != last) std::swap(first, current);
+  while (++current != last) std::swap(*first, *current);
 }
 
 template <typename I>

code/test_insertion_sort.cpp

@@ -1,21 +1,57 @@
 #include <functional>
 #include <iostream>
+#include <cassert>
 #include "insertion_sort.h"
+#include "list_pool.h"
 #include "algorithm.h"
 
 int main() {
+
+  // test binary insertion sort
   {
       int nums[] = {5, 3, 10, 1, 2};
-      print_range(nums, nums + 5);
       binary_insertion_sort_n(nums, 5, std::less<int>());
       print_range(nums, nums + 5);
+      assert(is_sorted(nums, nums + 5));
   }
+  // test linear insertion sort
   {
       int nums[] = {5, 3, 10, 1, 2};
-      print_range(nums, nums + 5);
       linear_insertion_sort_n(nums, 5, std::less<int>());
       print_range(nums, nums + 5);
+
+      assert(is_sorted(nums, nums + 5));
   }
+  // test for forward iterators
+  {
+      // std::list provides bidirectional iterators and
+      // std::forward_list is in C++11, so we will use
+      // our list pool to test this.
+      
+      list_pool<int, int> pool;
+
+      // Create a list "1 2 3",
+      typename list_pool<int, int>::list_type l = pool.end();
+      l = pool.allocate(3, l);
+      l = pool.allocate(2, l);
+      l = pool.allocate(1, l);
+
+      typedef typename list_pool<int, int>::iterator I;
+      I first(pool, l);
+      I last(pool);
+
+      // do a few rotates and print the results
+      rotate_right_by_one(first, last);
+      print_range(first, last);
+
+      rotate_right_by_one(first, last);
+      print_range(first, last);
+
+      rotate_right_by_one(first, last);
+      print_range(first, last);
+  }
+
+
   return 0;
 }