3
\$\begingroup\$

This project was part of the JetBrains Academy Java course. The project was about implementing an Encryption program that uses either a simple Caesar Cipher (or) a Unicode Cipher depending on the arguments being passed to the program. The data to be Encrypted/Decrypted is just one line.

The different arguments being passed are:

  1. "-mode": enc = Encryption, dec = Decryption.
  2. "-alg": shift = caesar, unicode = unicode.
  3. "-in": Name of from to read input from.
  4. "-data": Data to be Encrypted/Decrypted.
  5. "-out": File name to which we should write Encrypted/Decrypted message.
  6. "-key": key to Encrypt/Decrypt data.

Additional things to note:

  1. If both "-data" and "-in" arguments are given, consider the "-data" argument.
  2. If no "-out" argument is present, simply print out the result.
  3. If no "-alg" argument is given, consider it to be shift.

Code:

Main.java

package encryptdecrypt;

public class Main {
    public static void main(String[] args)  {
        String in = "", data = "", mode = "", alg = "shift", out = "";
        int key = 0;

        for (int i = 0; i < args.length; i++) {
            switch (args[i]) {
                case "-alg":
                    alg = args[i + 1];
                    break;

                case "-key":
                    key = Integer.parseInt(args[i + 1]);
                    break;

                case "-data":
                    data = args[i + 1];
                    break;

                case "-in":
                    in = args[i + 1];
                    break;

                case "-mode":
                    mode = args[i + 1];
                    break;

                case "-out":
                    out = args[i + 1];
                    break;
            }
        }


        if (alg.equals("unicode")) {
            EncryptDecrypt unicode = new UnicodeCipher();
            unicode.doDeEncryption(mode, data, in, out, key);
        } else {
            EncryptDecrypt shift = new ShiftCipher();
            shift.doDeEncryption(mode, data, in, out, key);
        }
    }
}

EncryptDecrypt.java

package encryptdecrypt;

import java.io.*;

public abstract class EncryptDecrypt {

    protected StringBuilder plainText = new StringBuilder();
    protected StringBuilder cipherText = new StringBuilder();

    public void doDeEncryption (String mode, String data, String in, String out, int key) {
        readFile(mode, data, in);
        if (mode.equals("enc")) {
            encryptPlainText(key);
        } else {
            decryptCipherText(key);
        }
        writeFile(mode, out);
    }

    public void readFile (String mode, String data, String in) {
        String temp = "";

        if (data.equals("")) {
            try {
                BufferedReader reader = new BufferedReader(new FileReader(in));
                temp = reader.readLine();
                reader.close();
            } catch (IOException e) {
                e.printStackTrace();
            }
        } else {
            temp = data;
        }

        if (mode.equals("enc")) {
            plainText = new StringBuilder(temp);
        } else {
            cipherText = new StringBuilder(temp);
        }
    }

    public void writeFile (String mode, String out) {
        String text = "";

        if (mode.equals("enc")) {
            text = cipherText.toString();
        } else {
            text = plainText.toString();
        }

        if (out.equals("")) {
            System.out.println(text);
            return;
        }

        try {
            BufferedWriter writer = new BufferedWriter(new FileWriter(out));
            writer.write(text);
            writer.close();
        } catch (IOException e) {
            e.printStackTrace();
        }
    }

    public abstract void encryptPlainText(int key);
    public abstract void decryptCipherText(int key);
}

ShiftCipher.java

package encryptdecrypt;

public class ShiftCipher extends EncryptDecrypt {

    private final int ROLLBACK = 26;

    private boolean isValid (char c) {
        return (((c >= 'a') && (c <= 'z')) || ((c >= 'A') && (c <= 'Z')));
    }
    @Override
    public void encryptPlainText(int key) {
        for (int i = 0; i < plainText.length(); i++) {
            if (isValid(plainText.charAt(i))) {
                int temp = (plainText.charAt(i) - 'a' + key) % ROLLBACK;
                temp += 'a';
                cipherText.append((char)temp);
            } else {
                cipherText.append(plainText.charAt(i));
            }
        }
    }

    @Override
    public void decryptCipherText(int key) {
        for (int i = 0; i < cipherText.length(); i++) {
            if (isValid(cipherText.charAt(i))) {
                int temp = (cipherText.charAt(i) - 'a' - key + ROLLBACK) % ROLLBACK;
                temp += 'a';
                plainText.append((char)temp);
            } else {
                plainText.append(cipherText.charAt(i));
            }
        }
    }
}

UnicodeCipher:

package encryptdecrypt;

public class UnicodeCipher extends EncryptDecrypt {
    @Override
    public void encryptPlainText(int key) {
        for (int i = 0; i < plainText.length(); i++) {
            int encryptedChar = (plainText.charAt(i) + key);
            cipherText.append((char)encryptedChar);
        }
    }

    @Override
    public void decryptCipherText(int key) {
        for (int i = 0; i < cipherText.length(); i++) {
            int decryptedChar = (cipherText.charAt(i) - key);
            plainText.append((char)decryptedChar);
        }
    }
}

The Unicode cipher is just adding the key to the codepoint of the character. The shift/Caesar Cipher must only encrypt the English letters.

Is my OOP structure good? Is there a better way to approach this using a design pattern?

\$\endgroup\$
0

1 Answer 1

2
\$\begingroup\$

Your code is very clearly written and formatted and it's easy to read. You have learned a few anti-patterns that I think you should try to unlearn.

Declaring variables in one line is not something that is usually done in common Java coding styles. It makes it harder to find the declarations.

Always initialising variables to a default special value defeats useful compiler warnings. If you leave them uninitialized, the compiler will warns you when you try to access an accidentally uninitialized field.

Using a special value as to mark "uninitialized" fields robs you from a useful use case: if a user wants to use the application as a part of a script, they cannot just use it to encrypt any string privided by some other program, they have to check if the value is empty first. Java has a universal value for uninitialized fields: null.

The field names are unnecessarily short. A field containing a file name should be named as such. So instead write your field declarations like this:

String inputFileName = null;
String outputFileName = null;
String inputData = null;

When you have a distinct set of input values, use enumerations to represent them in code instead of string constants:

private enum Mode {
    ENCRYPT,
    DECRYPT
}

And then...

Mode mode;

Command line parsin is tedious and boring. There are libraries for it, like Apache Commons-CLI. You are also dividing the command line parsing responsibility between the Main and the EncryptDecrypt classes by passing all the parameters parsed from the command line to EncryptDecrypt as such and making that class handle the error checking. You should try to follow the single responsibility principle and make the Main class be fully responsible for the command line and leave just the encryption to EncryptDecrypt. This will require rethinking of what the encryption algorithms need as input.

If you remove the responsibilities that are not strictly related to the encryption, you'll see that the encryption algorith only needs a single character and the encryption key as input.

public interface SubstitutionCipher {
    char encrypt(char ch);
}

The next step is figuring out how to pass file and command line data as input to the same cipher without forcing it to handle file IO. As the cipher works on text, one way would be to make the cipher operate on a Reader and convert the command line parameters to either FileReader or StringReader and FileWriter or StringWriter depending on input.

Then, to connect the Main class and the SubstitutionCipher you need a third class that reads a character from the reader, passes it to the cipher and writes the result to the writer.

Catching and logging exceptions in an application like below is not a good idea. The exception you catch here prevents the application from working, so you should end the application with an error message. So let the exception propagate to Main class, catch it, write an error to System.err and exit with an error code. This is the rare case where System.exit(int) should be used.

} catch (IOException e) {
    e.printStackTrace();
}

You have documented the command line parameters like "-mode" to have two choices but the code actually has one strict value and everything else defaults to the other. This is not a good idea as it causes invalid parameters like "-mode encode" to be interpreted as if the application was called with parameters "-mode dec".

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.