Skip to content

Conversation

@tojnya
Copy link

@tojnya tojnya commented Sep 5, 2024

The project is done.

Copy link
Contributor

@oleksandr-jr oleksandr-jr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This project is pure simplicity and elegance in good meaning.

  • Good documented code.
  • Cool solutions.
  • Sometimes the architecture might be more open for future improvements.

Live review:
https://youtu.be/qgbS_6g_D-w?si=jVurwICoEBpScE3E&t=3953

Comment on lines +16 to +29
for (int i = 0; i < text.length(); i++) {
char oneChar = text.charAt(i); // беремо символ з тексту
int index = Alphabet.ENGLISH_LOWERCASE.indexOf(oneChar); // шукаємо його в алфавіті lowercase
if (index != -1) { // якщо знайшли
result.append(rotatedLowercase.get(index)); // шифруємо і додаємо до результату
} else { // якщо немає
index = Alphabet.ENGLISH_UPPERCASE.indexOf(oneChar); // шукаємо його в алфавіті uppercase
if (index != -1) { // якщо знайшли
result.append(rotatedUppercase.get(index)); // шифруємо і додаємо до результату
} else { // якщо не знайшли в жодному з двох алфавітів, це спеціальний символ
result.append(text.charAt(i)); // НЕ шифруємо і просто додаємо
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be simplified.

Comment on lines +9 to +14
public static final ArrayList<Character> ENGLISH_UPPERCASE = new ArrayList<>(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'));
public static final ArrayList<Character> ENGLISH_LOWERCASE = new ArrayList<>(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'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could get rid of two lists and use only one by processing uppercase letters dynamically.

Comment on lines +42 to +51
for (String line : text) {
encrypted = line;
for (int i = 0; i < 26; i++) {
decrypted = decrypt(line, i);
if (checkForCommonWords(decrypted)) {
break;
}
}
break;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that you decrypt it using lines. In some cases, it's very effective, but there are also some cases when it might not work as expected. Maybe it's good idea to take more than 1 line if it contains not enough letters a.e. always use the text > 256 symbols.

Comment on lines +24 to +45
linesRead = fileManager.read(runOptions.getFilePath());
switch (runOptions.getCommand()) {
case ENCRYPT:
for (String lineRead : linesRead) {
linesWritten.add(cypher.encrypt(lineRead, runOptions.getKey()));
}
resultFile = fileManager.getNewPath(runOptions.getFilePath(), runOptions.getCommand());
break;
case DECRYPT:
for (String lineRead : linesRead) {
linesWritten.add(cypher.decrypt(lineRead, runOptions.getKey()));
}
resultFile = fileManager.getNewPath(runOptions.getFilePath(), runOptions.getCommand());
break;
case BRUTEFORCE:
int key = cypher.bruteForceKey(linesRead);
for (String lineRead : linesRead) {
linesWritten.add(cypher.decrypt(lineRead, key));
}
resultFile = fileManager.getNewPath(runOptions.getFilePath(), key);
break;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to keep the logic out of the main method.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.jar file shouldn't be committed. It should be added as a release executable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants