From f1022d54b432a58dd7251ea1a7572aea4cda7437 Mon Sep 17 00:00:00 2001
From: "Jake W. Ireland" <jakewilliami@icloud.com>
Date: Fri, 30 Apr 2021 11:38:39 +1200
Subject: [PATCH 1/2] Rotating functions optimise
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The two rotating functions that we have (for strings and for arrays) are not optimal, and I didn't quite pick up on this in [`b3acd44`](https://github.com/Smaug123/ClassicalCiphers.jl/commit/b3acd440f120a86d8b1b898ec30521cef7383ada).  This commit optimises them:
```julia
julia> using Benchmarktools, Random

julia> A = rand(1000);

julia> @btime rotate_right_old(A, 3);
  4.059 μs (1 allocation: 7.94 KiB)

julia> @btime rotate_right_new(A, 3);
  622.573 ns (1 allocation: 7.94 KiB)

julia> rotate_right_old(A, 3) == rotate_right_new(A, 3)
true

julia> @btime rotate_left_old(A, 3);
  4.042 μs (1 allocation: 7.94 KiB)

julia> @btime rotate_left_new(A, 3);
  639.320 ns (1 allocation: 7.94 KiB)

julia> rotate_left_old(A, 3) == rotate_left_new(A, 3)
true

julia> S = randstring(1000);

julia> @btime rotate_left_str_old(S, 3);
  30.889 μs (13 allocations: 10.55 KiB)

julia> @btime rotate_left(S, 3);
  883.661 ns (3 allocations: 2.16 KiB)

julia> rotate_left_str_old(S, 3) == rotate_left(S, 3)
true

julia> @btime rotate_right_str_old(S, 3);
  30.821 μs (13 allocations: 10.55 KiB)

julia> @btime rotate_right(S, 3);
  885.314 ns (3 allocations: 2.16 KiB)

julia> rotate_right_str_old(S, 3) == rotate_right(S, 3)
true
```
---
 src/common.jl | 32 ++++++++++----------------------
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/src/common.jl b/src/common.jl
index e50681f..244c547 100644
--- a/src/common.jl
+++ b/src/common.jl
@@ -3,30 +3,18 @@ function letters_only(text::AbstractString)
     return filter(x -> ('A' <= x <= 'Z' || 'a' <= x <= 'z'), text)
 end
 
-function rotate_right(arr::AbstractVector, n::Integer)
-    # implementation of the Mathematica function rotate_right - or you could try circshift()?
-    ans = copy(arr)
-    for i in 1:length(arr)
-        ans[i] = arr[((2*length(ans)+i-n-1) % length(ans)) + 1]
-    end
-    
-    return ans
+rotate_left(arr::AbstractVector, n::Integer) = circshift(arr, -n)
+rotate_right(arr::AbstractVector, n::Integer) = circshift(arr, n)
+
+function rotate_string(str::AbstractString, n::Integer)
+    i = mod(n, length(str))
+    return str[i+1:end] * str[1:i]
 end
 
-function rotate_left(arr::AbstractVector, n::Integer)
-    # implementation of the Mathematica function rotate_left
-    ans = copy(arr)
-    for i in 1:length(arr)
-        ans[i] = arr[((i + n - 1) % length(ans)) + 1]
-    end
-    
-    return ans
-end
-
-rotate_left_str(st::AbstractString, n::Integer) =
-    join(rotate_left(collect(st), n))
-rotate_right_str(st::AbstractString, n::Integer) =
-    join(rotate_right(collect(st), n))
+rotate_left(str::AbstractString, n::Integer) =
+    rotate_string(str, n)
+rotate_right(str::AbstractString, n::Integer) =
+    rotate_string(str, -n)
 
 function split_by(arr::AbstractVector, func::Function)
     # implementation of the Mathematica function split_by

From ed8d568a93fee826707512f6519493ceaeaa6a40 Mon Sep 17 00:00:00 2001
From: "Jake W. Ireland" <jakewilliami@icloud.com>
Date: Mon, 3 May 2021 18:50:47 +1200
Subject: [PATCH 2/2] Added more tests for code coverage goal

---
 test/common.jl   | 19 +++++++++++++++++++
 test/runtests.jl |  1 +
 2 files changed, 20 insertions(+)
 create mode 100644 test/common.jl

diff --git a/test/common.jl b/test/common.jl
new file mode 100644
index 0000000..ac9313b
--- /dev/null
+++ b/test/common.jl
@@ -0,0 +1,19 @@
+@test ClassicalCiphers.letters_only("Se9Wj8NK2:'n") == "SeWjNKn" 
+
+@test ClassicalCiphers.rotate_left([78, 18, 53, 96, 15, 35, 72, 29, 34, 26], 3) == [96, 15, 35, 72, 29, 34, 26, 78, 18, 53]
+@test ClassicalCiphers.rotate_right([78, 18, 53, 96, 15, 35, 72, 29, 34, 26], 3) == [29, 34, 26, 78, 18, 53, 96, 15, 35, 72]
+
+@test ClassicalCiphers.rotate_string("Se9Wj8NK2:'n", 5) == "8NK2:'nSe9Wj"
+@test ClassicalCiphers.rotate_left("Se9Wj8NK2:'n", 3) == "Wj8NK2:'nSe9"
+@test ClassicalCiphers.rotate_right("Se9Wj8NK2:'n", 3) == ":'nSe9Wj8NK2"
+
+@test ClassicalCiphers.split_by([78, 18, 53, 96, 15, 35, 72, 29, 34, 26], x -> x > 52) == [[78], [18], [53, 96], [15, 35], [72], [29, 34, 26]]
+
+# @test get_trigram_fitness() # I don't think we need this test because the function in question is immediately called and used in ../src/common.jl
+
+@test ClassicalCiphers.string_fitness("Se9Wj8NK2:'n") == 11.640550700535616
+
+# @test ClassicalCiphers.frequencies("hello") == Dict('h' => 1, 'e' => 1, 'l' => 2, 'o' => 1)
+
+@test ClassicalCiphers.index_of_coincidence("hello world") == 0.5777777777777777
+@test ClassicalCiphers.index_of_coincidence("smaug123classicalciphers") == 0.6190476190476191
diff --git a/test/runtests.jl b/test/runtests.jl
index 23b7857..6864fa8 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -2,6 +2,7 @@ include(joinpath(dirname(@__DIR__), "src", "ClassicalCiphers.jl")); using .Class
 using Test
 
 tests = [
+         "common",
          "playfair",
          "vigenere",
          "monoalphabetic",