Jump to content
Search In
  • More options...
Find results that contain...
Find results in...
  • entries
    52
  • comments
    48
  • views
    6,304

The Software Enigma Machine Bonus: Refactoring some code

Mira Yurizaki

58 views

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:

enigma_full.png

 

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:

 

image.png.cee4c09d602f5393496229516f5e48aa.png

 

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


Recommended Comments

There are no comments to display.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
×