HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Enigma Machine Simulation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
machineenigmasimulation

Problem

This is an Enigma Machine Simulator written in Java:

Enigma.java

```
package enigma;

import java.io.*;
import java.awt.*;
import java.awt.event.*;
import java.io.IOException;
import enigma.rotor.LargeRotor;
import enigma.rotor.MedRotor;
import enigma.rotor.SmallRotor;

public class Enigma extends Frame {
String FileExtension = ".enigma";

public static void main (String[] args) {
screen = new Enigma();
screen.show();
}

public static final int FrameWidth = 660;

public static final int FrameHeight = 400;

public static Enigma screen;

private final Insets ins;

private SmallRotor smrotor = new SmallRotor();
private MedRotor medrotor = new MedRotor();
private LargeRotor lgrotor = new LargeRotor();

protected TextArea Message = new TextArea();
protected TextArea Encrypted = new TextArea();

protected TextField messFN = new TextField();
protected TextField encFN = new TextField();

protected Label FNLabel = new Label("Enter a FileName",Label.CENTER );
protected Label FNLabel2 = new Label("Enter a FileName",Label.CENTER);

public Enigma() {
this.FileExtension = ".enigma";
setTitle ("Enigma Simulator");
setSize (FrameWidth, FrameHeight);
setResizable(false);

setLayout(new FlowLayout());

add(Message);
Panel p = new Panel();
p.setLayout(new GridLayout(5,1));
p.add(new ButtonAdapter("Encrypt"){@Override
public void pressed(){Encrypt();}});
p.add(FNLabel);
p.add(messFN);
p.add(new ButtonAdapter("Load"){@Override
public void pressed(){LoadMessage(messFN.getText());}});
p.add(new ButtonAdapter("Save"){@Override
public void pressed(){SaveMessage(messFN.getText());}});

add(p);

add(Encrypted);
Panel p2 = new Panel();
p2.setLayout(new GridLayout(5,1));
p2.add(new ButtonAdapter("Decrypt"){@Override
public void pressed(){Decrypt();}});
p2.add(FNLabel2);
p2.add(encFN);
p2.add(new But

Solution

Single responsibility principle

A class should be responsible for one thing. The Enigma class is doing too much. It paints a graphical user interface, it works with files, it encrypts and decrypts, and lots of other things. It would be good to split this up aggressively.

Start by moving everything out of the class that doesn't need a graphical element. Only keep buttons, panels, action listeners in Enigma. And then rename the class to EnigmaGUI or similar to better reflect its primary (and hopefully, the only) responsibility.

Use as much memory as you need and not more

In this code, the size of the cypher array appears to be arbitrary:

plain = plain.toUpperCase();
char [] cypher = new char[10000];

for(int i = 0; i < plain.length();i++)
    {
    cypher[i] = EncryptChar(plain.charAt(i));

    }


Isn't 10000 too much? Is it enough? You can eliminate such concerns by allocating exactly as much as you need:

char[] cypher = new char[plain.length()];


Code style

The formatting is really awful. I suggest to copy paste into an IDE like Eclipse or similar, use the auto-format feature, and see what that looks like. That's the format most java developers expect to see and find it easy to review.

Naming

The convention in Java is to use camelCase for variable and method names, and SHOUT_CASE for constants. It would greatly improve readability if you follow that.

Code Snippets

plain = plain.toUpperCase();
char [] cypher = new char[10000];

for(int i = 0; i < plain.length();i++)
    {
    cypher[i] = EncryptChar(plain.charAt(i));

    }
char[] cypher = new char[plain.length()];

Context

StackExchange Code Review Q#95089, answer score: 18

Revisions (0)

No revisions yet.