Your
Cell
struct should be immutable - you can achieve that by making all the fieldsreadonly
.In
Cell
character
should bePascalCase
.You repeat the code for trimming and replacing characters for they key and plain text. This should be moved into a method.
There is some convoluted code which iterates of the
keyWord
and adds stuff to akeyString
. If I read it correctly then what this is doing is: Build a list of all unique characters and remove those from the alphabet. LINQ is perfect for doing that in a concise way:var uniqueKeyChars = keyWord.Distinct().ToList(); alphabet = alphabet.Except(uniqueKeyChars).ToArray();
I would consider moving the whole pad-to-even-length and then split into pairs business into a separate methods as well. Also using a
Tuple
to represent pairs of characters seems to be more appropriate.public static IEnumerable<Tuple<char, char>> GetPairs(this IEnumerable<char> input) { while (input.Any()) { var pair = input.Take(2); char first = pair.First(); char second = pair.Skip(1).Any() ? pair.Last() : 'X'; yield return Tuple.Create(first, second); input = input.Skip(2); } }
Usage:
var plainTextPairs = plainText.GetPairs();
If you have followed the suggestions above you will now have a
uniqueKeyChars
array as well as thealphabet
with theuniqueKeyChars
removed. This should make building the matrix easier:var cellAlphabet = new Queue<char>(uniqueKeyChars.Concat(alphabet)); for (int x = 0; x < 5; x++) { for (int y = 0; y < 5; y++) { var cell = new Cell(cellAlphabet(cellAlphabet.Dequeue()), x, y); matrix.Add(cell); } }
When creating the cipher if you split the index finding from the appending then the code becomes slightly more readable (below assuming you use the extension method shown above):
foreach(string pair in plainTextPairs) { int indexA = matrix.FindIndex(c => c.character == pair.Item1); Cell a = matrix[indexA]; int indexB = matrix.FindIndex(c => c.character == pair.Item2); Cell b = matrix[indexB]; int cipherCellIndexA; int cipherCellIndexB; //Write cipher if (a.X == b.X) { cipherCellIndexA = matrix.FindIndex(c => c.X == (a.X + 1) % 5 && c.Y == a.Y); cipherCellIndexB = matrix.FindIndex(c => c.X == (b.X + 1)%5 && c.Y == b.Y); } else if(a.Y == b.Y) { cipherCellIndexA = matrix.FindIndex(c => c.X == a.X && c.Y == (a.Y + 1) % 5); cipherCellIndexB = matrix.FindIndex(c => c.X == b.X % 5 && c.Y == (b.Y + 1) % 5); }else { cipherCellIndexA = matrix.FindIndex(c => c.X == a.X && c.Y == b.Y); cipherCellIndexB = matrix.FindIndex(c => c.X == b.X % 5 && c.Y == a.Y); } cipher.Append(matrix[cipherCellIndexA].Character); cipher.Append(matrix[cipherCellIndexB].Character); }
Of course of you would make your matrix an actual 2d array then you could skip the whole
FindIndex
shebang and just pick the cell by the index which should speed up things a bit.
ChrisWue
- 20.4k
- 4
- 42
- 107