Skip to content

Зыбин - 3 tasks: encrypt, decrypt, bruteForce, consoleMenu#29

Open
ZyibinAV wants to merge 15 commits intodemologin:mainfrom
ZyibinAV:main
Open

Зыбин - 3 tasks: encrypt, decrypt, bruteForce, consoleMenu#29
ZyibinAV wants to merge 15 commits intodemologin:mainfrom
ZyibinAV:main

Conversation

@ZyibinAV
Copy link

@ZyibinAV ZyibinAV commented Sep 4, 2025

No description provided.

Copy link

@Khmelov Khmelov left a comment

Choose a reason for hiding this comment

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

Проект хорошо структурирован по пакетам, но в некоторых классах нужен рефакторинг. Выполнено три из четырёх видов шифрования и дешифрования, поставил оценку б


public static void main(String[] args) {

ConsoleMenuRunner runner = new ConsoleMenuRunner();
Copy link

Choose a reason for hiding this comment

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

В этой точке хорошая привычка передавать все или хотя бы некоторые зависимости приложения

@@ -0,0 +1,10 @@
package com.javarush.zybin.cipher;

public class Alphabet {
Copy link

Choose a reason for hiding this comment

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

Если это класс а не интерфейс то полезно добавить ему приватный конструктор

int maxMatches = 0;
String bestDecryption = "";

System.out.println(" \uD83D\uDE80 Starting brute force attack...");
Copy link

Choose a reason for hiding this comment

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

Во внутренних компонентах приложения допускается логирование, то есть вывод информации для разработчика, а пользовательский вывод лучше разместить на верхних уровнях

}
}
System.out.println("=".repeat(60));
System.out.println("Best key found: " + bestKey + " with " + maxMatches + " matches");
Copy link

Choose a reason for hiding this comment

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

То же самое


public class Decrypt {

public String action(String text, int key) {
Copy link

Choose a reason for hiding this comment

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

Данный способ передачи ограничен размерами памяти приложения, если бы это были потоки ввода и вывода, то данного ограничения не было бы

for (char character : text.toCharArray()) {
int index = -1;
int newIndex;
for (int i = 0; i < ALPHABET.length; i++) {
Copy link

Choose a reason for hiding this comment

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

Эти два метода очень сильно похожи друг на друга поэтому лучше вынести общий код в абстракцию


public ConsoleMenu() {
this.encrypt = new Encrypt();
this.decrypt = new Decrypt();
Copy link

Choose a reason for hiding this comment

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

Кажется я где-то видел второе создание экземпляр дешифровке, лучше бы взять его отсюда


public void handleEncryption() {
try {
System.out.println("Enter the encryption key (1-80)");
Copy link

Choose a reason for hiding this comment

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

В принципе все понятно но когда методы становятся большими лучше выделить из них дополнительные под методы с понятными именами, это делает код более читабельным


public void handleBruteForce() {
try {
System.out.println("Write the path to the file or the choice will occur by default");
Copy link

Choose a reason for hiding this comment

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

Очень перегруженный метод тут нужен рефакторинг

int choice = consoleMenu.getUserChoice();

switch (choice) {
case 1:
Copy link

Choose a reason for hiding this comment

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

Сложно отличить под смыслу эту единицу от той которая идёт ниже, возможно их лучше вынести в константы с понятными именами

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants