Performance considerations and addition of union of types

- Performance considerations, including better typing where possible; and - Addition of un-exported `AbstractPair` type for more general use of `playfair` functions
2025-10-05 17:38:48 +00:00 · 2021-01-08 02:03:06 +13:00
parent c4ebe4c0dc
commit b3acd440f1
10 changed files with 187 additions and 195 deletions
--- a/src/affine.jl
+++ b/src/affine.jl
@@ -10,12 +10,12 @@ Converts the input to uppercase, but retains symbols.
 Optional argument: offset=0, which specifies what number 'a' should be
 considered as.
 """
-function encrypt_affine(plaintext, mult::Integer, add::Integer; offset=0)
+function encrypt_affine(plaintext, mult::T, add::T; offset::T = 0) where {T <: Integer}
 	if mult % 2 == 0 || mult % 13 == 0
 		error("Multiplier must be coprime to 26.")
 	end

-	keystr = join([Char(((mult*i + add) % 26) + 97 - offset) for i in offset:25+offset], "")
+	keystr = join(Char[Char(((mult * i + add) % 26) + 97 - offset) for i in offset:(25 + offset)])
    encrypt_monoalphabetic(plaintext, keystr)
 end

@@ -31,16 +31,16 @@ Converts the input to lowercase, but retains symbols.
 Optional argument: offset=0, which specifies what number 'a' should be
 considered as.
 """
-function decrypt_affine(ciphertext, mult::Integer, add::Integer; offset=0)
+function decrypt_affine(ciphertext, mult::T, add::T; offset=0) where {T <: Integer}
 	if mult % 2 == 0 || mult % 13 == 0
 		error("Multiplier must be coprime to 26.")
 	end

-	keystr = join([Char(((mult*i + add) % 26) + 97 - offset) for i in offset:25+offset], "")
+	keystr = join(Char[Char(((mult*i + add) % 26) + 97 - offset) for i in offset:25+offset], "")
 	decrypt_monoalphabetic(ciphertext, keystr)
 end

-function max_by(arr, f)
+function max_by(arr::AbstractArray, f::Function)
 	currMax = undef
 	currAns = undef
 	set = false
@@ -69,11 +69,11 @@ Converts the input to lowercase, but retains symbols.
 Optional arguments: mult=0, which specifies the multiplier if known;
 add=-1, which specifies the additive constant if known.
 """
-function crack_affine(ciphertext::AbstractString; mult=0, add=-1)
-	mults = mult != 0 ? [mult] : [i for i in filter(x -> (x % 2 != 0 && x % 13 != 0), 1:25)]
-	adds = add != -1 ? [add] : (0:25)
+function crack_affine(ciphertext; mult::T = 0, add::T = -1) where {T <: Integer}
+	mults = mult != 0 ? Int[mult] : Int[i for i in filter(x -> (x % 2 != 0 && x % 13 != 0), 1:25)]
+	adds = add != -1 ? Int[add] : (0:25)

 	possible_keys = Iterators.product(mults, adds)

-	reverse(max_by([(i, decrypt_affine(ciphertext, i[1], i[2])) for i in possible_keys], x -> string_fitness(x[2])))
+	reverse(max_by(Tuple{eltype(possible_keys), AbstractString}[(i, decrypt_affine(ciphertext, i[1], i[2])) for i in possible_keys], x -> string_fitness(x[2])))
 end
--- a/src/caesar.jl
+++ b/src/caesar.jl
@@ -5,10 +5,10 @@ so encrypt_caesar("abc", 1) == "BCD".

 Converts the input to uppercase.
 """
-function encrypt_caesar(plaintext, key::T) where {T<:Integer}
+function encrypt_caesar(plaintext, key::T) where {T <: Integer}
  # plaintext: string; key: integer offset, so k=1 sends "a" to "b"
-  key = ((key-1) % 26) + 1
-  keystr = join([collect(Char(97+key):'z'); collect('a':Char(97+key-1))])
+  key = ((key - 1) % 26) + 1
+  keystr = join(vcat(collect(Char(97 + key):'z'), collect('a':Char(97 + key - 1))))
  encrypt_monoalphabetic(plaintext, keystr)
 end

@@ -19,10 +19,10 @@ so decrypt_caesar("abcd", 1) == "zabc".

 Converts the input to lowercase.
 """
-function decrypt_caesar(ciphertext, key::Integer)
+function decrypt_caesar(ciphertext, key::T) where {T <: Integer}
  # ciphertext: string; key: integer offset, so k=1 decrypts "B" as "a"
-  key = ((key-1) % 26) + 1
-  lowercase(encrypt_caesar(ciphertext, 26-key))
+  key = ((key - 1) % 26) + 1
+  lowercase(encrypt_caesar(ciphertext, 26 - key))
 end

 """
@@ -35,8 +35,8 @@ With cleverness=1, maximises the string's total fitness.

 Converts the input to lowercase.
 """
-function crack_caesar(ciphertext; cleverness=1)
-  texts = [(decrypt_caesar(ciphertext,key), key) for key in 0:25]
+function crack_caesar(ciphertext; cleverness::T = 1) where {T <: Integer}
+  texts = Tuple{String, Int}[(decrypt_caesar(ciphertext,key), key) for key in 0:25]
  if cleverness == 1
    texts = sort(texts, by=(x -> string_fitness(first(x))))
  else
--- a/src/common.jl
+++ b/src/common.jl
@@ -1,9 +1,9 @@
-function letters_only(text)
+function letters_only(text::AbstractString)
  # text: string; removes all non-alphabetic characters
  filter(x -> ('A' <= x <= 'Z' || 'a' <= x <= 'z'), text)
 end

-function rotateRight(arr, n)
+function rotateRight(arr::AbstractVector, n::T) where {T <: Integer}
  # implementation of the Mathematica function RotateRight - or you could try circshift()?
  ans = copy(arr)
  for i in 1:length(arr)
@@ -12,31 +12,31 @@ function rotateRight(arr, n)
  ans
 end

-function rotateLeft(arr, n)
+function rotateLeft(arr::AbstractVector, n::T) where {T <: Integer}
  # implementation of the Mathematica function RotateLeft
  ans = copy(arr)
  for i in 1:length(arr)
-    ans[i] = arr[((i+n-1) % length(ans)) + 1]
+    ans[i] = arr[((i + n - 1) % length(ans)) + 1]
  end
  ans
 end

-function rotateLeftStr(st::AbstractString, n)
-  join(rotateLeft(split(st, ""), n), "")
+function rotateLeftStr(st::AbstractString, n::T) where {T <: Integer}
+  join(rotateLeft(collect(st), n))
 end

-function rotateRightStr(st::AbstractString, n)
-  join(rotateRight(split(st, ""), n), "")
+function rotateRightStr(st::AbstractString, n::T) where {T <: Integer}
+  join(rotateRight(collect(st), n))
 end

-function splitBy(arr, func)
+function splitBy(arr::AbstractVector, func::Function)
  # implementation of the Mathematica function SplitBy
  # splits the array into sublists so that each list has the same value of func
  # on its elements
  arrinternal = map(func, arr)
  currans = Vector{Integer}[[arr[1]]]
  for i in 2:length(arr)
-    if arrinternal[i] != arrinternal[i-1]
+    if arrinternal[i] != arrinternal[i - 1]
      append!(currans, Vector{Integer}[[arr[i]]])
    else
      append!(currans[end], [arr[i]])
@@ -45,30 +45,28 @@ function splitBy(arr, func)
  currans
 end

-function get_trigram_fitnesses()
+function get_trigram_fitnesses(datafile::AbstractString)
  # The quadgrams file we use is licensed MIT, as per
  # http://practicalcryptography.com/cryptanalysis/text-characterisation/quadgrams/#comment-2007984751
-  f = open(joinpath(dirname(Base.source_path()), "english_trigrams.txt"))
-  lines = readlines(f)
-  dict = Dict{AbstractString, Integer}()
+  dict = Dict{AbstractString, Int32}()
  
-  for l in lines
-    (ngram, fitness) = split(l)
-    dict[ngram] = parse(Int32, fitness)
+  open(datafile) do io
+    while ! eof(io)
+      (ngram, fitness) = split(readline(io))
+      dict[ngram] = parse(Int32, fitness)
+    end
  end
-
-  close(f)
  dict
 end

-trigram_fitnesses = get_trigram_fitnesses()
+trigram_fitnesses = get_trigram_fitnesses(joinpath(dirname(Base.source_path()), "english_trigrams.txt"))

 """
 Performs a trigram analysis on the input string, to determine how close it
 is to English. That is, splits the input string into groups of three letters,
 and assigns a score based on the frequency of the trigrams in true English.
 """
-function string_fitness(input::AbstractString; alreadystripped=false)
+function string_fitness(input::AbstractString; alreadystripped::Bool = false)
  if !alreadystripped
    str = letters_only(input)
  else
@@ -79,24 +77,25 @@ function string_fitness(input::AbstractString; alreadystripped=false)

  ans = 0
  for i in 1:(length(str)-2)
-    ans += get(trigram_fitnesses, str[i:i+2], 0)
+    ans += get(trigram_fitnesses, str[i:(i + 2)], 0)
  end

-  log(ans/length(str))
+  log(ans / length(str))
 end

 """
 Finds the frequencies of all characters in the input string, returning a Dict
 of 'a' => 4, for instance. Uppercase characters are considered distinct from lowercase.
 """
-function frequencies(input)
-  ans = Dict{Char, Integer}()
-  for i in input
-    if haskey(ans, i)
-      ans[i] += 1
-    else
-      ans[i] = 0
-    end
+function frequencies(input::AbstractString)
+	ans = Dict{Char, Int}()
+  for c in input
+	  index = Base.ht_keyindex2!(ans, c)
+	  if index > 0
+      @inbounds ans.vals[index] += 1
+	  else
+      @inbounds Base._setindex!(ans, 0, c, -index)
+	  end
  end
  ans
 end
@@ -105,14 +104,14 @@ end
 Finds the index of coincidence of the input string. Uppercase characters are considered to be
 equal to their lowercase counterparts.
 """
-function index_of_coincidence(input)
+function index_of_coincidence(input::AbstractString)
  freqs = frequencies(lowercase(letters_only(input)))
  len = length(lowercase(letters_only(input)))

  ans = 0
  for i in 'a':'z'
-    ans += (x -> x*(x-1))(get(freqs, i, 0))
+		ans += map(x -> x * (x - 1), get(freqs, i, 0))
  end

-  ans /= (len * (len-1) / 26)
+  ans /= (len * (len - 1) / 26)
 end
--- a/src/enigma.jl
+++ b/src/enigma.jl
@@ -1,25 +1,25 @@
 import Base.uppercase

-function uppercase(a::Tuple{Char, Char})
-	(uppercase(a[1]), uppercase(a[2]))
+function uppercase(a::NTuple{N, Char}) where N
+	uppercase.(a)
 end

 function parse_stecker(stecker::AbstractString)
-	if length(stecker) % 2 != 0
+	if !iseven(length(stecker))
 		error("Stecker setting must be of even length.")
 	end

-	if stecker == ""
+	if isempty(stecker)
 		steck_parsed = Tuple{Char, Char}[]
 	else
-		sp = split(stecker, "")
-		steck_parsed = [(sp[i][1], sp[i+1][1]) for i in 1:2:length(sp)]
+		sp = collect(stecker)
+		steck_parsed = Tuple{Char, Char}[(sp[i][1], sp[i + 1][1]) for i in 1:2:length(sp)]
 	end
 	steck_parsed
 end

 function parse_stecker(stecker::Array{Tuple{Char, Char}})
-	if stecker == []
+	if isempty(stecker)
 		return Array{Tuple{Char, Char}, 1}()
 	else
 		return stecker
@@ -45,7 +45,7 @@ function parse_reflector(reflector::AbstractString)

 	ans = uppercase(reflector)

-	if ans != join(unique(ans), "")
+	if ans != join(unique(ans))
 		error("Reflector must not contain any character used more than once.")
 	end

@@ -76,17 +76,17 @@ function encrypt_enigma(plaintext,
 						rotors::Array{T, 1}, key::AbstractString;
 						reflector_id='B', ring::AbstractString = "AAA",
 						stecker = Tuple{Char, Char}[],
-						skip_stecker_check = false) where {T<:Integer}
+						skip_stecker_check = false) where {T <: Integer}
 	parsed_stecker = parse_stecker(stecker)
 	# validate stecker settings
 	if !skip_stecker_check
-		if collect(Iterators.flatten(parsed_stecker)) != collect(unique(Iterators.flatten(parsed_stecker)))
+		# if collect(Iterators.flatten(parsed_stecker)) != collect(unique(Iterators.flatten(parsed_stecker)))
+		if collect(parsed_stecker) != unique(parsed_stecker) # do we need to flatten? ^
 			error("No letter may appear more than once in stecker settings.")
 		end
 	end
 	parsed_stecker::Array{Tuple{Char, Char}} = map(uppercase, parsed_stecker)

-
 	# validate ring settings
 	if length(ring) != 3
 		error("Ring settings must be a string of length 3.")
@@ -126,13 +126,12 @@ function encrypt_enigma(plaintext,
 	plaintext = uppercase(letters_only(plaintext))

 	# initialisation of the machine
-
 	rotor_layouts = ["EKMFLGDQVZNTOWYHXUSPAIBRCJ",
 					 "AJDKSIRUXBLHWTMCQGZNPYFVOE",
 					 "BDFHJLCPRTXVZNYEIWGAKMUSQO",
 					 "ESOVPZJAYQUIRHXLNFTGKDCMWB",
 					 "VZBRGITYUPSDNHLXAWMJQOFECK"]
-	notches = [17,5,22,10,26]
+	notches = Int[17,5,22,10,26]

 	rotor1 = keystr_to_dict(rotor_layouts[rotors[1]])
 	notch1 = notches[rotors[1]]
@@ -141,25 +140,24 @@ function encrypt_enigma(plaintext,
 	rotor3 = keystr_to_dict(rotor_layouts[rotors[3]])
 	notch3 = notches[rotors[3]]

-	rotor1_inv = Dict{Char, Char}([reverse(a) for a in rotor1])
-	rotor2_inv = Dict{Char, Char}([reverse(a) for a in rotor2])
-	rotor3_inv = Dict{Char, Char}([reverse(a) for a in rotor3])
+	rotor1_inv = Dict{Char, Char}(Pair{Char, Char}[reverse(a) for a in rotor1])
+	rotor2_inv = Dict{Char, Char}(Pair{Char, Char}[reverse(a) for a in rotor2])
+	rotor3_inv = Dict{Char, Char}(Pair{Char, Char}[reverse(a) for a in rotor3])

 	# apply the key as part of initialisation; incorporates ring
-	key_offsets = [26+Int(ch)-65 for ch in key]
-	notch1 = (key_offsets[1]*26+notch1-key_offsets[1]) % 26
-	notch2 = (key_offsets[2]*26+notch2-key_offsets[2]) % 26
-	notch3 = (key_offsets[3]*26+notch3-key_offsets[3]) % 26
+	key_offsets = Int[26 + Int(ch) - 65 for ch in key]
+	notch1 = (key_offsets[1] * 26 + notch1 - key_offsets[1]) % 26
+	notch2 = (key_offsets[2] * 26 + notch2 - key_offsets[2]) % 26
+	notch3 = (key_offsets[3] * 26 + notch3 - key_offsets[3]) % 26

-	key_offsets = key_offsets .- [Int(ring[i])-65 for i in 1:3]
+	key_offsets = key_offsets .- Int[Int(ring[i]) - 65 for i in 1:3]

 	# We receive a character; the rotors increment; then:
 	# the character goes through the plugboard
 	# the character then goes through rotor3, then rotor2, then rotor1
 	# then the reflector, then the inverse of rotor 1, 2, 3
 	# finally the plugboard again
-
-	plugboard_dict = Dict([parsed_stecker; map(reverse, parsed_stecker)])
+	plugboard_dict = Dict(vcat(parsed_stecker, map(reverse, parsed_stecker)))

    ans = IOBuffer()

@@ -167,10 +165,7 @@ function encrypt_enigma(plaintext,
 	rotor2movements = key_offsets[2]
 	rotor1movements = key_offsets[1]

-	for i in 1:length(plaintext)
-
-		working_ch = plaintext[i]
-
+	for (i, working_ch) in enumerate(plaintext)
 		# rotate rotors
 		notch3 -= 1
 		rotor3movements += 1
@@ -203,42 +198,42 @@ function encrypt_enigma(plaintext,


 		# plugboard
-		working_ch = encrypt_monoalphabetic(working_ch, plugboard_dict)[1]
+		working_ch = first(encrypt_monoalphabetic(working_ch, plugboard_dict))

 		# rotors
 		# comes in as…
-		working_ch = Char(65+((rotor3movements+Int(working_ch)-65) % 26))
-		working_ch = encrypt_monoalphabetic(working_ch, rotor3)[1]
+		working_ch = Char(65 + ((rotor3movements + Int(working_ch) - 65) % 26))
+		working_ch = first(encrypt_monoalphabetic(working_ch, rotor3))

 		# comes in as…
- 		working_ch = Char(65+(((26*rotor3movements)-rotor3movements+rotor2movements+Int(working_ch)-65) % 26))
+ 		working_ch = Char(65 + (((26 * rotor3movements) - rotor3movements + rotor2movements + Int(working_ch) - 65) % 26))
 		working_ch = encrypt_monoalphabetic(working_ch, rotor2)[1]

 		# comes in as…
- 		working_ch = Char((((26*rotor2movements) + Int(working_ch)-65 - rotor2movements + rotor1movements) % 26) + 65)
-		working_ch = encrypt_monoalphabetic(working_ch, rotor1)[1]
+ 		working_ch = Char((((26 * rotor2movements) + Int(working_ch) - 65 - rotor2movements + rotor1movements) % 26) + 65)
+		working_ch = first(encrypt_monoalphabetic(working_ch, rotor1))

 		# reflector
 		# comes in as…
-		working_ch = Char((26*rotor1movements + Int(working_ch) - 65 - rotor1movements) % 26 + 65)
-		working_ch = encrypt_monoalphabetic(working_ch, reflector)[1]
+		working_ch = Char((26 * rotor1movements + Int(working_ch) - 65 - rotor1movements) % 26 + 65)
+		working_ch = first(encrypt_monoalphabetic(working_ch, reflector))

 		# rotors
 		# comes in as…
-		working_ch = Char((Int(working_ch)-65+rotor1movements) % 26 + 65)
+		working_ch = Char((Int(working_ch) - 65 + rotor1movements) % 26 + 65)

 		# we use encrypt_monoalphabetic and inverse-dictionaries already computed, for speed,
 		# where it is more natural to use decrypt_monoalphabetic
-		working_ch = uppercase(encrypt_monoalphabetic(working_ch, rotor1_inv))[1]
- 		working_ch = Char(65+((rotor1movements*26 + rotor2movements - rotor1movements +Int(working_ch)-65) % 26))
-		working_ch = uppercase(encrypt_monoalphabetic(working_ch, rotor2_inv))[1]
- 		working_ch = Char(65+((26*rotor2movements + rotor3movements-rotor2movements+Int(working_ch)-65) % 26))
-		working_ch = uppercase(encrypt_monoalphabetic(working_ch, rotor3_inv))[1]
+		working_ch = uppercase(first(encrypt_monoalphabetic(working_ch, rotor1_inv)))
+ 		working_ch = Char(65 + ((rotor1movements * 26 + rotor2movements - rotor1movements + Int(working_ch) - 65) % 26))
+		working_ch = uppercase(first(encrypt_monoalphabetic(working_ch, rotor2_inv)))
+ 		working_ch = Char(65 + ((26 * rotor2movements + rotor3movements - rotor2movements + Int(working_ch) - 65) % 26))
+		working_ch = uppercase(first(encrypt_monoalphabetic(working_ch, rotor3_inv)))

 		# plugboard
 		# comes in as…
-		working_ch = Char(65+(((26*rotor3movements)-rotor3movements+Int(working_ch)-65) % 26))
-		working_ch = encrypt_monoalphabetic(working_ch, plugboard_dict)[1]
+		working_ch = Char(65 + (((26 * rotor3movements) - rotor3movements + Int(working_ch) - 65) % 26))
+		working_ch = first(encrypt_monoalphabetic(working_ch, plugboard_dict))

 		print(ans, working_ch)
 	end
--- a/src/hill.jl
+++ b/src/hill.jl
@@ -12,7 +12,7 @@ is thrown.

 The matrix must be invertible modulo 26. If it is not, an error is thrown.
 """
-function encrypt_hill(plaintext::AbstractString, key::AbstractArray{T, 2}) where {T<:Integer}
+function encrypt_hill(plaintext::AbstractString, key::AbstractArray{T, 2}) where {T <: Integer}
 	if round(Integer, det(key)) % 26 == 0
 		error("Key must be invertible mod 26.")
 	end
@@ -21,15 +21,13 @@ function encrypt_hill(plaintext::AbstractString, key::AbstractArray{T, 2}) where
    text = uppercase(letters_only(plaintext))

 	# split the plaintext up into that-size chunks
-
 	# pad if necessary
    if length(text) % keysize != 0
    	text = text * ("X"^(keysize - (length(text) % keysize)))
    end
-    chars = [Int(ch[1]) - 65 for ch in split(text, "")]
+    chars = Int[Int(ch[1]) - 65 for ch in text]
    # split
    split_text = reshape(chars, (keysize, div(length(text), keysize)))
-
    encrypted = mapslices(group -> 65 .+ ((key * group) .% 26), split_text, dims = [1])

    ans = IOBuffer()
@@ -43,7 +41,6 @@ end


 function encrypt_hill(plaintext::AbstractString, key::AbstractString)
-
    if round(Integer, sqrt(length(key))) != sqrt(length(key))
    	error("Hill key must be of square integer size.")
    end
@@ -56,16 +53,16 @@ function encrypt_hill(plaintext::AbstractString, key::AbstractString)
 	encrypt_hill(plaintext, transpose(key_matrix))
 end

-function minor(mat::AbstractArray{T, 2}, i, j) where {T<:Integer}
-	d = det(mat[[1:i-1; i+1:end], [1:j-1; j+1:end]])
+function minor(mat::AbstractArray{T, 2}, i::K, j::K) where {T <: Integer, K <: Integer}
+	d = det(mat[vcat(1:(i - 1), (i + 1):end), vcat(1:(j - 1), (j + 1):end)])
 	round(Integer, d)
 end

 """
 Computes the adjugate matrix for given matrix.
 """
-function adjugate(mat::AbstractArray{T, 2}) where {T<:Integer}
-	arr = [(-1)^(i+j) * minor(mat, i, j) for (i, j) in Iterators.product(1:size(mat, 1), 1:size(mat, 2))]
+function adjugate(mat::AbstractArray{T, 2}) where {T <: Integer}
+	arr = Integer[(-1)^(i+j) * minor(mat, i, j) for (i, j) in Iterators.product(1:size(mat, 1), 1:size(mat, 2))]
 	ans = reshape(arr, size(mat))
 	Array{Integer, 2}(transpose(ans))
 end
@@ -88,14 +85,12 @@ function decrypt_hill(ciphertext, key::AbstractArray{T, 2}) where {T<:Integer}
 end

 function decrypt_hill(ciphertext, key::AbstractString)
-
 	if round(Integer, sqrt(length(key))) != sqrt(length(key))
    	error("Hill key must be of square integer size.")
    end

 	matrix_dim = round(Integer, sqrt(length(key)))
 	keys = map(x -> Int(x) - 65, collect(uppercase(letters_only(key))))
-
 	key_matrix = reshape(keys, matrix_dim, matrix_dim)

 	decrypt_hill(ciphertext, transpose(key_matrix))
--- a/src/monoalphabetic.jl
+++ b/src/monoalphabetic.jl
@@ -1,5 +1,5 @@
 function keystr_to_dict(keystr::AbstractString)
-  Dict{Char, Char}(map(x -> (Char(x[1]+64), x[2]), enumerate(uppercase(keystr))))
+  Dict{Char, Char}(map(x -> (Char(x[1] + 64), x[2]), enumerate(uppercase(keystr))))
 end

 """
@@ -13,9 +13,9 @@ once, and the string is converted to lowercase.
 If the key is given as a Dict, the only substitutions made are those in the Dict;
 in particular, the string is not converted to lowercase automatically.
 """
-function encrypt_monoalphabetic(plaintext, key::Dict)
+function encrypt_monoalphabetic(plaintext, key::Dict{Char, Char})
  # plaintext: string; key: dictionary of {'a' => 'b'}, etc, for replacing 'a' with 'b'
-  join([(i in keys(key) ? key[i] : i) for i in plaintext], "")
+  join(eltype(keys(key))[get(key, i, i) for i in plaintext])
 end

 """
@@ -29,7 +29,7 @@ once, and the string is converted to lowercase.
 If the key is given as a Dict, the only substitutions made are those in the Dict;
 in particular, the string is not converted to lowercase automatically.
 """
-function decrypt_monoalphabetic(ciphertext, key::Dict)
+function decrypt_monoalphabetic(ciphertext, key::Dict{Char, Char})
  # ciphertext: string; key: dictionary of {'a' => 'b'}, etc, where the plaintext 'a' was
  # replaced by ciphertext 'b'. No character should appear more than once
  # as a value in {key}.
@@ -47,7 +47,7 @@ function decrypt_monoalphabetic(ciphertext, key::AbstractString)
  # working in lowercase; key is assumed only to have each element appearing once
  # and to be in lowercase
  # so decrypt_monoalphabetic("cb", "cbade…") is "ab"
-  dict = Dict(a => Char(96 + findfirst(i -> i == a, lowercase(key))) for a in lowercase(key))
+  dict = Dict{Char, Char}(a => Char(96 + findfirst(i -> i == a, lowercase(key))) for a in lowercase(key))
  encrypt_monoalphabetic(lowercase(ciphertext), dict)
 end

@@ -66,7 +66,7 @@ function swap_two(str)
    indices = rand(1:length(str), 2)
  end

-  join([i == indices[1] ? str[indices[2]] : (i == indices[2] ? str[indices[1]] : str[i]) for i in 1:length(str)], "")
+  join(Integer[i == indices[1] ? str[indices[2]] : (i == indices[2] ? str[indices[1]] : str[i]) for i in 1:length(str)])
 end

 """
@@ -89,13 +89,13 @@ acceptance_prob=((e, ep, t) -> ep>e ? 1 : exp(-(e-ep)/t)), which is the probabil
  with which we accept new key of fitness ep, given that the current key has fitness e,
  at temperature t.
 """
-function crack_monoalphabetic(ciphertext; starting_key="",
-                              min_temp=0.0001, temp_factor=0.97,
-                              acceptance_prob=((e,ep,t) -> ep > e ? 1. : exp(-(e-ep)/t)),
-                              chatty=0,
-                              rounds=1)
+function crack_monoalphabetic(ciphertext; starting_key::AbstractString = "",
+                              min_temp::F = 0.0001, temp_factor::F = 0.97,
+                              acceptance_prob::F = ((e,ep,t) -> ep > e ? 1. : exp(-(e-ep)/t)),
+                              chatty::T = 0,
+                              rounds::T = 1) where {T <: Integer, F <: AbstractFloat}

-  if starting_key == ""
+  if isempty(starting_key)
  # most common letters
    commonest = "ETAOINSHRDLUMCYWFGBPVKZJXQ"
    freqs = frequencies(uppercase(letters_only(ciphertext)))
@@ -105,13 +105,13 @@ function crack_monoalphabetic(ciphertext; starting_key="",
      end
    end

-    freqs_input = sort(collect(freqs), by = tuple -> last(tuple), rev=true)
+    freqs_input = sort(collect(freqs), by = tuple -> last(tuple), rev = true)
    start_key = fill('a', 26)
    for i in 1:26
-      start_key[Int(commonest[i])-64] = freqs_input[i][1]
+      start_key[Int(commonest[i]) - 64] = first(freqs_input[i])
    end

-    key = join(start_key, "")
+    key = join(start_key)
  else
    key = starting_key
  end
@@ -122,16 +122,15 @@ function crack_monoalphabetic(ciphertext; starting_key="",

  stripped_ciphertext = letters_only(ciphertext)
  fitness = string_fitness(decrypt_monoalphabetic(stripped_ciphertext, key))
-  total_best_fitness = fitness
-  total_best_key = key
+  total_best_fitness, total_best_key = fitness, key
  total_best_decrypt = decrypt_monoalphabetic(ciphertext, key)

  for roundcount in 1:rounds
-    temp = 10^((roundcount-1)/rounds)
+    temp = 10^((roundcount - 1) / rounds)
    while temp > min_temp
-      for i in 1:round(Int, min(ceil(1/temp), 10))
+      for i in 1:round(Int, min(ceil(1 / temp), 10))
        neighbour = swap_two(key)
-        new_fitness = string_fitness(decrypt_monoalphabetic(stripped_ciphertext, neighbour), alreadystripped=true)
+        new_fitness = string_fitness(decrypt_monoalphabetic(stripped_ciphertext, neighbour), alreadystripped = true)
        if new_fitness > total_best_fitness
          total_best_fitness = new_fitness
          total_best_key = neighbour
@@ -163,8 +162,7 @@ function crack_monoalphabetic(ciphertext; starting_key="",
      end
    end

-    key = total_best_key
-    fitness = total_best_fitness
+    key, fitness = total_best_key, total_best_fitness
    temp = 1
  end

--- a/src/playfair.jl
+++ b/src/playfair.jl
@@ -1,28 +1,34 @@
+AbstractPair{F, S} = Union{Tuple{F, S}, Pair{F, S}}
+parse_abstract_pair(P::AbstractPair) =
+	P isa Tuple{Char, Char} ? Dict(reverse(Pair(P...))) : Dict(reverse(P))
+
 """
 Converts the given key-string to a Playfair key square.

-Parameter `replacement` is a pair, such as ('I', 'J'), containing
+Parameter `replacement` is a pair, such as ('I', 'J') or 'I' => 'J', containing
 the two letters which are combined. Only the first of these letters will
 be present in the keysquare.
 """
-function playfair_key_to_square(key::AbstractString, replacement)
+function playfair_key_to_square(key::AbstractString, replacement::AbstractPair{Char, Char})
 	# make the key replacement
-	key = encrypt_monoalphabetic(key, Dict(replacement[2] => replacement[1]))
+	D = parse_abstract_pair(replacement)
+	key = encrypt_monoalphabetic(key, D)
 	# delete duplicates etc from key
 	key_sanitised = union(uppercase(letters_only(key)))
 	# construct key square
-	remaining = filter(x -> (x != replacement[2] && !(in(x, key_sanitised))), 'A':'Z')
-	keysquare = reshape(collect(Iterators.flatten([key_sanitised; remaining])), 5, 5)
-    return permutedims(keysquare, (2, 1)) # transpose() is deprecated
+	remaining = filter(x -> (x != last(replacement) && !(in(x, key_sanitised))), 'A':'Z')
+	# keysquare = reshape(collect(Iterators.flatten(vcat(key_sanitised, remaining))), 5, 5)
+	keysquare = reshape(vcat(key_sanitised, remaining), 5, 5) # do we need to flatten? ^
+    return permutedims(keysquare, (2, 1)) # transpose() is deprecated for higher dimensional arrays
 end

-function encrypt_playfair(plaintext, key::AbstractString; combined=('I','J'))
+function encrypt_playfair(plaintext, key::AbstractString; combined::AbstractPair{Char, Char} = ('I','J'))
 	keysquare = playfair_key_to_square(key, combined)
-
+	D = parse_abstract_pair(combined)
 	# make combinations in plaintext
-	plaintext_sanitised = encrypt_monoalphabetic(plaintext, Dict(combined[2] => combined[1]))
+	plaintext_sanitised = encrypt_monoalphabetic(plaintext, D)

-	encrypt_playfair(plaintext_sanitised, keysquare, combined=combined)
+	encrypt_playfair(plaintext_sanitised, keysquare, combined = combined)
 end

 """
@@ -37,42 +43,43 @@ stripped=false. When set to true, encrypt_playfair skips
 combined=('I', 'J'), marks the characters which are to be combined in the text.
  Only the first of these two may be present in the output of encrypt_playfair.
 """
-function encrypt_playfair(plaintext, key::Array{Char, 2}; stripped=false, combined=('I', 'J'))
+function encrypt_playfair(plaintext, key::Array{Char, 2}; stripped::Bool = false, combined::AbstractPair{Char, Char} = ('I', 'J'))
 	if !stripped
-		if in(combined[2], key)
-			error("Key must not contain symbol $(combined[2]), as it was specified to be combined.")
+		if in(last(combined), key)
+			error("Key must not contain symbol $(last(combined)), as it was specified to be combined.")
 		end
+		D = parse_abstract_pair(combined)
 		plaintext_sanitised = uppercase(letters_only(plaintext))
-		plaintext_sanitised = encrypt_monoalphabetic(plaintext_sanitised, Dict(combined[2] => combined[1]))
+		plaintext_sanitised = encrypt_monoalphabetic(plaintext_sanitised, D)
 	else
 		plaintext_sanitised = plaintext
 	end

 	# add X's as necessary to break up double letters
-	if combined[2] != 'X'
+	if last(combined) != 'X'
 		padding_char = 'X'
 	else
-		padding_char = combined[1]
+		padding_char = first(combined)
 	end
-	if combined[2] != 'Z'
+	if last(combined) != 'Z'
 		backup_padding_char = 'Z'
 	else
-		backup_padding_char = combined[1]
+		backup_padding_char = first(combined)
 	end

 	i = 1
 	while i < length(plaintext_sanitised)
-      if plaintext_sanitised[i] == plaintext_sanitised[i+1]
+      if plaintext_sanitised[i] == plaintext_sanitised[i + 1]
      	if plaintext_sanitised[i] != padding_char
-      		plaintext_sanitised = plaintext_sanitised[1:i] * string(padding_char) * plaintext_sanitised[i+1:end]
+      		plaintext_sanitised = plaintext_sanitised[1:i] * string(padding_char) * plaintext_sanitised[(i + 1):end]
      	else
-      		plaintext_sanitised = plaintext_sanitised[1:i] * string(backup_padding_char) * plaintext_sanitised[i+1:end]
+      		plaintext_sanitised = plaintext_sanitised[1:i] * string(backup_padding_char) * plaintext_sanitised[(i + 1):end]
      	end
      end
      i += 2
    end

-    if length(plaintext_sanitised) % 2 == 1
+    if isodd(length(plaintext_sanitised))
    	if plaintext_sanitised[end] != padding_char
    		plaintext_sanitised = plaintext_sanitised * string(padding_char)
    	else
@@ -86,7 +93,7 @@ function encrypt_playfair(plaintext, key::Array{Char, 2}; stripped=false, combin
    i = 1
    while i < length(plaintext_sanitised)
    	l1 = plaintext_sanitised[i]
-    	l2 = plaintext_sanitised[i+1]
+    	l2 = plaintext_sanitised[i + 1]

    	l1pos = CartesianIndices((5, 5))[findfirst(i -> i == l1, key)]
    	l2pos = CartesianIndices((5, 5))[findfirst(i -> i == l2, key)]
@@ -94,11 +101,11 @@ function encrypt_playfair(plaintext, key::Array{Char, 2}; stripped=false, combin
    	@assert l1pos != l2pos

    	if l1pos[1] == l2pos[1]
-    		print(ans, key[l1pos[1], (((l1pos[2]+1 - 1) % 5) + 1)])
-    		print(ans, key[l2pos[1], (((l2pos[2]+1 - 1) % 5) + 1)])
+    		print(ans, key[l1pos[1], (((l1pos[2] + 1 - 1) % 5) + 1)])
+    		print(ans, key[l2pos[1], (((l2pos[2] + 1 - 1) % 5) + 1)])
    	elseif l1pos[2] == l2pos[2]
-    		print(ans, key[((l1pos[1]+1 - 1) % 5)+1, l1pos[2]])
-    		print(ans, key[((l2pos[1]+1 - 1) % 5)+1, l2pos[2]])
+    		print(ans, key[((l1pos[1] + 1 - 1) % 5) + 1, l1pos[2]])
+    		print(ans, key[((l2pos[1] + 1 - 1) % 5) + 1, l2pos[2]])
    	else

    		print(ans, key[l1pos[1], l2pos[2]])
@@ -112,9 +119,9 @@ function encrypt_playfair(plaintext, key::Array{Char, 2}; stripped=false, combin
 end


-function decrypt_playfair(ciphertext, key::AbstractString; combined=('I', 'J'))
+function decrypt_playfair(ciphertext, key::AbstractString; combined::AbstractPair{Char, Char} = ('I', 'J'))
 	keysquare = playfair_key_to_square(key, combined)
-	decrypt_playfair(ciphertext, keysquare, combined=combined)
+	decrypt_playfair(ciphertext, keysquare, combined = combined)
 end

 """
@@ -122,8 +129,8 @@ Decrypts the given ciphertext according to the Playfair cipher.

 Does not attempt to delete X's inserted as padding for double letters.
 """
-function decrypt_playfair(ciphertext, key::Array{Char, 2}; combined=('I', 'J'))
+function decrypt_playfair(ciphertext, key::Array{Char, 2}; combined::AbstractPair{Char, Char} = ('I', 'J'))
 	# to obtain the decrypting keysquare, reverse every row and every column
-	keysquare = reverse(reverse(key, dims=1), dims=2)
-	lowercase(encrypt_playfair(ciphertext, keysquare, combined=combined))
+	keysquare = rot180(key)
+	lowercase(encrypt_playfair(ciphertext, keysquare, combined = combined))
 end
--- a/src/portas.jl
+++ b/src/portas.jl
@@ -8,7 +8,7 @@ Converts the text to uppercase.
 function encrypt_portas(plaintext, key_in::AbstractString)
  key = uppercase(letters_only(key_in))
  plaintext = uppercase(plaintext)
-  keyarr = [div(Int(ch) - 65, 2) for ch in key]
+  keyarr = Int[div(Int(ch) - 65, 2) for ch in key]

  keycounter = 1
  ans = IOBuffer()
--- a/src/solitaire.jl
+++ b/src/solitaire.jl
@@ -1,11 +1,11 @@
-function next_solitaire(deckIn::AbstractVector{T}) :: AbstractVector{Integer} where {T<:Integer}
+function next_solitaire(deckIn::AbstractVector{T}) where {T <: Integer}
  # performs one round of Solitaire on the given deck
 # first joker
  deck = deckIn
  jokerPos = findfirst(i -> i == 53, deck)
  if jokerPos != length(deck)
-    inter = deck[jokerPos+1]
-    deck[jokerPos+1] = deck[jokerPos]
+    inter = deck[jokerPos + 1]
+    deck[jokerPos + 1] = deck[jokerPos]
    deck[jokerPos] = inter
  else
    inter = deck[end]
@@ -17,11 +17,11 @@ function next_solitaire(deckIn::AbstractVector{T}) :: AbstractVector{Integer} wh
  jokerPos = findfirst(i -> i == 54, deck)
  if jokerPos <= length(deck) - 2
    inter = deck[jokerPos]
-    deck[jokerPos] = deck[jokerPos+1]
+    deck[jokerPos] = deck[jokerPos + 1]
    deck[jokerPos + 1] = deck[jokerPos + 2]
    deck[jokerPos + 2] = inter
  elseif jokerPos == length(deck) - 1
-    inter = deck[length(deck)-1]
+    inter = deck[length(deck) - 1]
    inter1 = deck[length(deck)]
    deck[end] = deck[1]
    deck[length(deck)-1] = inter1
@@ -33,12 +33,11 @@ function next_solitaire(deckIn::AbstractVector{T}) :: AbstractVector{Integer} wh
    deck[1] = deck[2]
    deck[2] = inter1
    deck[end] = inter
-    deck = rotateRight(deck,1)
+    deck = rotateRight(deck, 1)
  end
 # triple-cut
  split_deck = splitBy(deck, x -> x > 52)
  if deck[1] > 52 && deck[end] > 52
-    1
    # do nothing
  elseif deck[1] > 52
    split_deck = rotateRight(split_deck, 1)
@@ -51,19 +50,19 @@ function next_solitaire(deckIn::AbstractVector{T}) :: AbstractVector{Integer} wh
  end
  deck = collect(Iterators.flatten(split_deck))
 # take bottom of deck and put it just above last card
-  cardsToTake = (deck[end] > 52) ? 0 : deck[end]
+  cardsToTake = deck[end] > 52 ? 0 : deck[end]

-  intermediate = rotateLeft(deck[1:length(deck)-1], cardsToTake)
+  intermediate = rotateLeft(deck[1:length(deck) - 1], cardsToTake)
  append!(intermediate, [deck[end]])
  deck = intermediate

  return collect(deck)
 end

-function keychar_from_deck(deck::AbstractVector{T}) :: Integer where {T<:Integer}
+function keychar_from_deck(deck::AbstractVector{T}) where {T <: Integer}
  # given a deck, returns an integer which is the Solitaire key value
  # output by that deck
-  deck[((deck[1]==54 ? 53 : deck[1]) % length(deck)) + 1]
+  deck[((deck[1] == 54 ? 53 : deck[1]) % length(deck)) + 1]
 end

 struct SolitaireKeyStreamStruct
@@ -82,7 +81,7 @@ function Base.iterate(b::SolitaireKeyStreamStruct, state)
  (keychar_from_deck(curState)::Integer, next_solitaire(curState))
 end

-function SolitaireKeyStream(initialDeck::AbstractVector{T}) where {T<:Integer}
+function SolitaireKeyStream(initialDeck::AbstractVector{T}) where {T <: Integer}
  Iterators.filter(i -> i <= 52, Iterators.drop(SolitaireKeyStreamStruct(initialDeck), 1))
 end

@@ -92,10 +91,10 @@ The key may be given either as a vector initial deck, where the cards are
 1 through 54 (the two jokers being 53, 54), or as a string.
 Schneier's keying algorithm is used to key the deck if the key is a string.
 """
-function encrypt_solitaire(string, initialDeck::AbstractVector{T}) :: AbstractString where {T<:Integer}
+function encrypt_solitaire(string::AbstractString, initialDeck::AbstractVector{T}) where {T <: Integer}
  inp = uppercase(letters_only(string))
  ans = ""
-  for (keyval::Integer, input::Char) in zip(SolitaireKeyStream(initialDeck), collect(inp))
+  for (keyval, input) in zip(SolitaireKeyStream(initialDeck), collect(inp))
    ans *= encrypt_caesar(input, keyval)
  end
  return ans
@@ -107,7 +106,7 @@ The key may be given either as a vector initial deck, where the cards are
 1 through 54 (the two jokers being 53, 54), or as a string.
 Schneier's keying algorithm is used to key the deck if the key is a string.
 """
-function decrypt_solitaire(string, initialDeck::Vector)
+function decrypt_solitaire(string::AbstractString, initialDeck::AbstractVector{T}) where {T <: Integer}
  inp = uppercase(letters_only(string))
  ans = ""
  i = 0
@@ -117,13 +116,13 @@ function decrypt_solitaire(string, initialDeck::Vector)
  return ans
 end

-function key_deck(key::AbstractString) :: AbstractVector{Integer}
+function key_deck(key::AbstractString)
  # returns the Solitaire deck after it has been keyed with the given string
  deck = collect(1:54)
  for keyval in uppercase(letters_only(key))
    deck = next_solitaire(deck)
-    cardsToTake = Int(keyval)-64
-    intermediate = rotateLeft(deck[1:length(deck)-1], cardsToTake)
+    cardsToTake = Int(keyval) - 64
+    intermediate = rotateLeft(deck[1:(length(deck) - 1)], cardsToTake)
    append!(intermediate, [deck[end]])
    deck = intermediate
  end
@@ -135,6 +134,6 @@ function encrypt_solitaire(string::AbstractString, key::AbstractString) :: Abstr
  encrypt_solitaire(string, key)
 end

-function decrypt_solitaire(string, key::AbstractString)
+function decrypt_solitaire(string::AbstractString, key::AbstractString)
  decrypt_solitaire(string, key_deck(key))
 end
--- a/src/vigenere.jl
+++ b/src/vigenere.jl
@@ -6,8 +6,8 @@ For example, encrypt_vigenere("ab", [0, 1]) returns "AC".
 """
 function encrypt_vigenere(plaintext, key::Array)
  # plaintext: string; key: vector of integer offsets, so [0, 1] encrypts "ab" as "ac"
-  ans = [encrypt_caesar(chr, key[(i-1) % length(key)+1]) for (i, chr) in enumerate(letters_only(plaintext))]
-  join(ans, "")
+  ans = String[encrypt_caesar(chr, key[(i - 1) % length(key) + 1]) for (i, chr) in enumerate(letters_only(plaintext))]
+  join(ans)
 end

 """
@@ -16,7 +16,7 @@ For example, decrypt_vigenere("ac", [0, 1]) returns "ab".
 """
 function decrypt_vigenere(ciphertext, key::Array)
  # ciphertext: string; key: vector of integer offsets, so [0, 1] decrypts "ac" as "ab"
-  lowercase(encrypt_vigenere(ciphertext, map(x -> 26-x, key)))
+  lowercase(encrypt_vigenere(ciphertext, map(x -> 26 - x, key)))
 end

 """
@@ -25,7 +25,7 @@ For example, encrypt_vigenere("ab", "ab") returns "AC".
 """
 function encrypt_vigenere(ciphertext, key::AbstractString)
  # ciphertext: string; key: string, so "ab" encrypts "ab" as "AC"
-  encrypt_vigenere(ciphertext, [Int(i)-97 for i in lowercase(letters_only(key))])
+  encrypt_vigenere(ciphertext, Int[Int(i) - 97 for i in lowercase(letters_only(key))])
 end

 """
@@ -34,7 +34,7 @@ For example, decrypt_vigenere("ab", "ac") returns "ab".
 """
 function decrypt_vigenere(plaintext, key::AbstractString)
  # plaintext: string; key: string, so "ab" decrypts "ac" as "ab"
-  decrypt_vigenere(plaintext, [Int(i)-97 for i in lowercase(letters_only(key))])
+  decrypt_vigenere(plaintext, Int[Int(i) - 97 for i in lowercase(letters_only(key))])
 end

 """
@@ -47,15 +47,15 @@ keylength=0: if the key length is known, specifying it may help the solver.
  If 0, the solver will attempt to derive the key length using the index
  of coincidence.
 """
-function crack_vigenere(plaintext; keylength=0)
+function crack_vigenere(plaintext; keylength::Integer = 0)
  stripped_text = letters_only(lowercase(plaintext))
  if keylength == 0
-  	lens = sort(collect(2:15), by= len -> mean([index_of_coincidence(stripped_text[i:len:end]) for i in 1:len]))
+  	lens = sort(collect(2:15), by = len -> mean(AbstractFloat[index_of_coincidence(stripped_text[i:len:end]) for i in 1:len]))
    keylength = lens[end]
  end

-  everyother = [stripped_text[i:keylength:end] for i in 1:keylength]
-  decr = [crack_caesar(st)[1] for st in everyother]
+  everyother = String[stripped_text[i:keylength:end] for i in 1:keylength]
+  decr = String[crack_caesar(st)[1] for st in everyother]

  ans = IOBuffer()
  for column in 1:length(decr[1])
@@ -66,7 +66,6 @@ function crack_vigenere(plaintext; keylength=0)
  	end
  end

-  derived_key = join([Char(65+crack_caesar(st)[2]) for st in everyother], "")
+  derived_key = join(Char[Char(65 + crack_caesar(st)[2]) for st in everyother])
  (derived_key, String(take!(ans)))
-
 end