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

Playing around with Vigenere and Caesar cipher - Java command line encryption program

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

Problem

I came along these two ciphers in my cryptography book and though I'd implement them just for fun. After that I went on developing a cipher based on these two that minimize the weakness of each cipher alone.
I would appreciate your expert opinions on it and if it would be possible to crypt-analyse.

```
import java.util.*;
import java.io.*;

class CaesarCipher{
static String encrypt(String s, int key){
List arr = Arrays.asList('A','B','C','D','E','F','G','H','I','J','K','L','M','N','O','P','Q','R','S','T','U','V','W','X','Y','Z');
StringBuilder str = new StringBuilder();
int index;
for(int i = 0; i arr = Arrays.asList('A','B','C','D','E','F','G','H','I','J','K','L','M','N','O','P','Q','R','S','T','U','V','W','X','Y','Z',' ','0','1','2','3','4','5','6','7','8','9');
Random rand = new Random(h);
StringBuilder str = new StringBuilder();
int index1, index2;
int hash = h;
String key = k;
for(int i = 0; i arr = Arrays.asList('A','B','C','D','E','F','G','H','I','J','K','L','M','N','O','P','Q','R','S','T','U','V','W','X','Y','Z',' ','0','1','2','3','4','5','6','7','8','9');
Random rand = new Random(h);
StringBuilder str = new StringBuilder();
int index1, index2;
int hash = h;
String key = k;
String input = buff.readLine();
int count = 0;
while(input != null){
for(int i = 0; i arr = Arrays.asList('A','B','C','D','E','F','G','H','I','J','K','L','M','N','O','P','Q','R','S','T','U','V','W','X','Y','Z',' ','0','1','2','3','4','5','6','7','8','9');
Random rand = new Random(h);
StringBuilder str = new StringBuilder();
int index1, index2;
int hash = h;
String key = k;
for(int i = 0; i " + arr.get((index1 + (arr.size() - index2) )% arr.size()));
}
}

return str.toString();
}

static String decryptFile(BufferedReader buff, String k,int h) throws IOException{
List arr = Arrays.asList('A','B','C','D','E','F','G','H','I','J','K','L','M','N','O','P','Q','R','S','T','U','V','W','X','Y','Z',' ','0','1','2','3','4','5','6','7','8','9');
Random rand = new Random(h);
StringBuilder str

Solution

Indentation and braces style

The indentation is frankly messy, and I don't think it's entirely due to the Markdown formatting.

Also, please be consistent in your braces style. For example:

if(arr.contains(Character.toUpperCase(s.charAt(i))) == false)
    str.append(s.charAt(i));
else {
    index = arr.indexOf(Character.toUpperCase(s.charAt(i)));
    str.append(arr.get(Math.abs(index + key) % arr.size()));
}


There's no need to skim on the extra { and } characters, and without them it makes scanning the code slightly harder, IMHO.

Negating if conditions

if(arr.contains(Character.toUpperCase(s.charAt(i))) == false) { /* ... */ }


That can be better expressed in the following way without using == false:

if(!arr.contains(Character.toUpperCase(s.charAt(i)))) { /* ... */ }


Variables names

In VigenereCaesar:

static String encrypt(String s, String k, int h){ /* ... */ }


It's not immediately obvious what these variables represent. Using the full form is recommended.

Command-line parsing

You may want to consider using some third-party libraries to help you parse command-line options, instead of doing yourself.

try-with-resource

You should also be using try-with-resource once for safe and efficient handling of your Scanner instance and the underlying System.in input stream. For example:

public static void main(String[] args) {
    try (Scanner scanner = new Scanner(System.in)) {
        String mode = getMode(scanner);
        if (mode.equals("encrypt")) {
            String key = getKey(scanner);
            String hash = getHash(scanner);
            // do something with key and hash
        } else if (mode.equals("decrypt")) {
            // similar to above
        } else {
            // Indicate input error
        }
    }
}


In the methods getMode(Scanner), getKey(Scanner) and getHash(Scanner), they can simply call the appropriate methods to get user input, and throws IOException to propagate any I/O exceptions back to the main() method. Validation errors should be handled within the methods though, so that they can be expected to return one result.

Constant List collections

You are declaring Arrays.asList('A', 'B', 'C', / ... / 'Z') repetitively. You can consider putting it as an unmodifiable public static final List collection, and then only for the case where you want to shuffle it, create a new instance based on it to do so.

Code Snippets

if(arr.contains(Character.toUpperCase(s.charAt(i))) == false)
    str.append(s.charAt(i));
else {
    index = arr.indexOf(Character.toUpperCase(s.charAt(i)));
    str.append(arr.get(Math.abs(index + key) % arr.size()));
}
if(arr.contains(Character.toUpperCase(s.charAt(i))) == false) { /* ... */ }
if(!arr.contains(Character.toUpperCase(s.charAt(i)))) { /* ... */ }
static String encrypt(String s, String k, int h){ /* ... */ }
public static void main(String[] args) {
    try (Scanner scanner = new Scanner(System.in)) {
        String mode = getMode(scanner);
        if (mode.equals("encrypt")) {
            String key = getKey(scanner);
            String hash = getHash(scanner);
            // do something with key and hash
        } else if (mode.equals("decrypt")) {
            // similar to above
        } else {
            // Indicate input error
        }
    }
}

Context

StackExchange Code Review Q#134031, answer score: 2

Revisions (0)

No revisions yet.