Java implementation of the caesar-cipher

Question

I've wrote a little program that encrypts text by using the caesar-cipher. Also it contains a little GUI, created by using swing. Here's the full code:

import javax.swing.JOptionPane;
import javax.swing.JScrollPane;
import javax.swing.JTextArea;
import java.awt.Dimension;
public class caesar {

    public static void main(String[] args) {
        String field, text;

        field = JOptionPane.showInputDialog("Please enter text:");
        field = field.replaceAll("[^a-zA-Z]+", "");
        field = field.toUpperCase();

        int shift;

        String shift_String = JOptionPane.showInputDialog("Please enter shift to the right:");
        shift = Integer.parseInt(shift_String);

        String d = JOptionPane.showInputDialog("Encrypt (1) or decrypt (2):");
        int decision = Integer.parseInt(d);

        String out;


        if(decision==1) {
            out = encrypt(field, shift);
            JTextArea msg = new JTextArea(out);
            msg.setLineWrap(true);
            msg.setWrapStyleWord(true);
            JScrollPane scrollPane = new JScrollPane(msg);
            scrollPane.setPreferredSize(new Dimension(300,300));
            JOptionPane.showMessageDialog(null, scrollPane);
        }
        if(decision==2) {
            out = decrypt(field, shift);
            JTextArea msg = new JTextArea(out);
            msg.setLineWrap(true);
            msg.setWrapStyleWord(true);
            JScrollPane scrollPane = new JScrollPane(msg);
            scrollPane.setPreferredSize(new Dimension(300,300));
            JOptionPane.showMessageDialog(null, scrollPane);
        }
    }

    //Encryption
    public static String encrypt(String text, int n) {
        int x = 0;
        int y = 0;
        String out = ""; //Empty string for result.
        while (x < text.length()) {
            if (text.charAt(x) > 64 && text.charAt(x) < 91) {
                if (text.charAt(x) + n > 90) {
                    y = 26;
                }
                out = out + (char) (text.charAt(x) + n - y);
            } else {
                out = out + text.charAt(x);
            }
            x++;
            y = 0;
        }
        return out;
    }

    //Decryption
    public static String decrypt(String text, int n) {
        int x = 0;
        int y = 0;
        String out = ""; //Empty string for result.
        while (x < text.length()) {
            if (text.charAt(x) > 64 && text.charAt(x) < 91) {
                if (text.charAt(x)-n < 65) {
                    y = 26;
                }
                out = out + (char) (text.charAt(x) - n + y);
            } else {
                out = out + text.charAt(x);
            }
            x++;
            y = 0;
        }
        return out;
    }
}

My question now is: How to improve this code?

I mean, it does what it is supposed to do, but it's not really great code.

Answer 1

In my opinion, the main problem with your code is the duplication, here is my advices.

1) Put the ui code out of the conditions.

The only issue there, if the choice is invalid, you can either show a default string, or throw an exception.

        if (decision == 1) {
            out = encrypt(field, shift);
        } else if (decision == 2) {
            out = decrypt(field, shift);
        } else {
            out = "Invalid choice!";
        }

        JTextArea msg = new JTextArea(out);
        msg.setLineWrap(true);
        msg.setWrapStyleWord(true);
        JScrollPane scrollPane = new JScrollPane(msg);
        scrollPane.setPreferredSize(new Dimension(300, 300));
        JOptionPane.showMessageDialog(null, scrollPane);

Or

        if (decision == 1) {
            out = encrypt(field, shift);
        } else if (decision == 2) {
            out = decrypt(field, shift);
        } else {
            throw new IllegalStateException("Invalid choice!")
        }

        JTextArea msg = new JTextArea(out);
        msg.setLineWrap(true);
        msg.setWrapStyleWord(true);
        JScrollPane scrollPane = new JScrollPane(msg);
        scrollPane.setPreferredSize(new Dimension(300, 300));
        JOptionPane.showMessageDialog(null, scrollPane);

2) In the encrypt & decrypt, to create the string containing the result, i suggest that you use java.lang.StringBuilder instead of concatening the String; you will gain some performance.

    public static String decrypt(String text, int n) {
        int x = 0;
        int y = 0;
        StringBuilder out = new StringBuilder(); //Empty string for result.
        while (x < text.length()) {
            if (text.charAt(x) > 64 && text.charAt(x) < 91) {
                if (text.charAt(x) - n < 65) {
                    y = 26;
                }
                out.append(text.charAt(x) - n + y);
            } else {
                out.append(text.charAt(x));
            }
            x++;
            y = 0;
        }
        return out.toString();
    }

3) In the encrypt & decrypt, extract the text.charAt(x) in a variable, to remove the duplicates.

    public static String decrypt(String text, int n) {
        int x = 0;
        int y = 0;
        StringBuilder out = new StringBuilder(); //Empty string for result.
        while (x < text.length()) {
            final char currentChar = text.charAt(x);

            if (currentChar > 64 && currentChar < 91) {
                if (currentChar - n < 65) {
                    y = 26;
                }
                out.append(currentChar - n + y);
            } else {
                out.append(currentChar);
            }
            x++;
            y = 0;
        }
        return out.toString();
    }

4) The encrypt and decrypt methods are pretty similar, you can probably merge them if you want.

    //Encryption
    public static String encrypt(String text, int n) {
        return operation(text, n, true);
    }

    //Decryption
    public static String decrypt(String text, int n) {
        return operation(text, n, false);
    }

    public static String operation(String text, int n, boolean isEncryption) {
        int x = 0;
        int y = 0;
        StringBuilder out = new StringBuilder(); //Empty string for result.
        while (x < text.length()) {
            final char currentChar = text.charAt(x);

            if (currentChar > 64 && currentChar < 91) {
                if (isEncryption ? (currentChar + n > 90) : (currentChar - n < 65)) {
                    y = 26;
                }
                out.append(isEncryption ? (currentChar + n - y) : (currentChar - n + y));
            } else {
                out.append(currentChar);
            }
            x++;
            y = 0;
        }
        return out.toString();
    }
```

Answer 2

something in addition:

• stick with code style guides. (e.g. I prefer: https://google.github.io/styleguide/javaguide.html .Something got in my eye: public class caesar, shift_String)
• validate users input and notify if something is wrong (it would more user friendly and safe)
• assign 'magic' numbers to a constants (in encrypt/decrypt methods, will add more readability to the code)
• avoid code duplication (encrypt/decrypt and decision bodies)

an example of how main could look like:

public static void main(String[] args) {
    String inputText = JOptionPane.showInputDialog("Please enter text:");
    String normalizedInput = normalizeText(inputText);

    int shiftBy = getIntFromInput("Please enter shift to the right:");
    int option = getIntFromInput("Encrypt (1) or decrypt (2):"); // todo for options i'd recommend to use Enum

    // todo arg validation example
    String resultMessage;
    switch (option) {
        case 1:
            resultMessage = encrypt(normalizedInput, shiftBy);
            break;
        case 2:
            resultMessage = decrypt(normalizedInput, shiftBy);
            break;
        default:
            resultMessage = "Unsupported option: " + option;
    }
    showDialogWithMessage(resultMessage);
}

private static String normalizeText(String inputText) {
    return inputText
            .replaceAll("[^a-zA-Z]+", "")
            .toUpperCase();
}

private static int getIntFromInput(String message) {
    return Integer.parseInt(JOptionPane.showInputDialog(message));
}

private static void showDialogWithMessage(String message) {
    JTextArea msg = new JTextArea(message);
    msg.setLineWrap(true);
    msg.setWrapStyleWord(true);
    JScrollPane scrollPane = new JScrollPane(msg);
    scrollPane.setPreferredSize(new Dimension(300, 300));
    JOptionPane.showMessageDialog(null, scrollPane);
}