The Software Enigma Machine Bonus: Refactoring some code
A bonus update to the Software Enigma Machine I did way back when. This time, I went back and refactored some code because I felt it needed it.
The Outline
-
Part 1
- What is the Enigma Machine?
- Why did I choose to implement the Enigma machine?
-
Before Programming Begins:
- Understanding the theory of the Enigma Machine
- Finding a programming environment
-
Part 2
- Programming the features
- Rotors
- Rotor housing
-
Part 3
- Plug Board
-
Part 4
- GUI
If you'd like to look at the source code, it's uploaded on GitHub.
What needed refactoring?
One of the things I did was placed each individual element on the Full version of the GUI, which to recap looked like this:
The problem was that placing everything down and updating all of the properties took forever. But this isn't the only way to add GUI elements onto a Windows Form. You can programmatically add them in! So I went about to figure out how to do this and after a few minutes of experimentation, I came up wit this GUI in the designer:
The idea here is that I have "anchor points" for the lights, keys, and plug board. Based on these anchor points I can then build the rest of the elements as needed. Picking on the "lights", the old way of doing things was this snippet of code:
private void setupLampLabels() { lampLabels = new List<Label>(); lampLabels.Add(ALightLabel); lampLabels.Add(BLightLabel); lampLabels.Add(CLightLabel); lampLabels.Add(DLightLabel); lampLabels.Add(ELightLabel); lampLabels.Add(FLightLabel); lampLabels.Add(GLightLabel); lampLabels.Add(HLightLabel); lampLabels.Add(ILightLabel); lampLabels.Add(JLightLabel); lampLabels.Add(KLightLabel); lampLabels.Add(LLightLabel); lampLabels.Add(MLightLabel); lampLabels.Add(NLightLabel); lampLabels.Add(OLightLabel); lampLabels.Add(PLightLabel); lampLabels.Add(QLightLabel); lampLabels.Add(RLightLabel); lampLabels.Add(SLightLabel); lampLabels.Add(TLightLabel); lampLabels.Add(ULightLabel); lampLabels.Add(VLightLabel); lampLabels.Add(WLightLabel); lampLabels.Add(XLightLabel); lampLabels.Add(YLightLabel); lampLabels.Add(ZLightLabel); }
Ick. All this was doing was adding each object to the lampLabels list. By switching it to a programmatically generated one, the code now looks like this:
static string[] QWERTY_LINE = {"QWERTYUIO", "ASDFGHJK", "ZXCVBNMLP"}; private void setupLightLabels() { int row = 0; lampLabels = new List<Label>(); setupLight(QLightLabel, row++); setupLight(ALightLabel, row++); setupLight(ZLightLabel, row++); lastLight = QLightLabel; lampLabels.Sort((x, y) => String.Compare(x.Text, y.Text)); } private void setupLight(Label BaseLabel, int Row) { const int LABEL_SPACING = 36; lampLabels.Add(BaseLabel); for (int Col = 1; Col < QWERTY_LINE[Row].Length; Col++) { Label lampLabel = new Label(); lampLabel.Font = BaseLabel.Font; lampLabel.TextAlign = ContentAlignment.MiddleCenter; lampLabel.BackColor = Color.Gray; lampLabel.Parent = fullMethodTabPage; lampLabel.Location = new Point(BaseLabel.Location.X + (Col * LABEL_SPACING), BaseLabel.Location.Y); lampLabel.Size = new Size(30, 30); lampLabel.Text = QWERTY_LINE[Row][Col].ToString(); lampLabel.Name = string.Format("{0}LightLabel", QWERTY_LINE[Row][Col]); lampLabels.Add(lampLabel); } }
What this does is builds a row of lights based on an anchor point. In this case, It builds off the Q, A, and Z lights. This makes a few things much easier to do now:
- If I wanted to change how the letters are laid out, I can update the QWERTY_LINE array.
- If I wanted to change where and how the lights themselves are laid out, I only have to move their anchor point around.
- I only have to move the anchor points around, I don't have to move a bunch of GUI widgets around. Also this generates uniform spacing too.
This does come with the caveat that each string in the QWERTY_LINE array has to start with an anchor letter.
The second refactoring I did was how the plug board shuffling works. Before it was this confusing mess that to be honest, I can't really explain it anymore off the top of my head:
public void ShuffleWiring() { int entries = mapping.Count; Dictionary<string, string>.KeyCollection keys = mapping.Keys; for(int i = 0; i < entries; i++) { int randEntry = rng.Next(entries + 1); int counter = 0; string firstLetter = ""; string secondLetter = ""; foreach (string randomKey in keys) { firstLetter = randomKey; counter++; if (counter == randEntry) break; } randEntry = (rng.Next() % entries); counter = 0; foreach (string randomKey in keys) { secondLetter = randomKey; counter++; if (counter == randEntry) break; } ChangeWiring(firstLetter, secondLetter); } }
I found one key thing that made this method much shorter: You can take a dictionary's keys and turn them into an array if you use System.Linq. Now this method becomes:
public void ShuffleWiring() { string[] keys = mapping.Keys.ToArray(); for (int i = 0; i < keys.Length; i++) { string firstLetter = keys[i]; string secondLetter = keys[rng.Next(keys.Length)]; ChangeWiring(firstLetter, secondLetter); } }
And it's much easier to understand.
I also did some other miscellaneous refactoring such as integrating utility methods that only had one reference into the method that called it. Another is I got rid of this method:
private void resetLights() { foreach(Label lamp in lampLabels) lamp.BackColor = Color.Gray; }
I didn't like this because this was referenced a few times and each time it was referenced, it went through all 26 lights to reset the background color. So instead of doing that, I have a lastLight object that holds a reference to the last light that was touched. Then, whenever this method was called, it was replaced with lastLamp.BackColor = Color.Gray;
0 Comments
There are no comments to display.