Skip to content

Martynov 4 tasks completed#17

Open
ViktorMartynov92 wants to merge 2 commits intodemologin:mainfrom
ViktorMartynov92:main
Open

Martynov 4 tasks completed#17
ViktorMartynov92 wants to merge 2 commits intodemologin:mainfrom
ViktorMartynov92:main

Conversation

@ViktorMartynov92
Copy link

No description provided.

Copy link
Owner

@demologin demologin left a comment

Choose a reason for hiding this comment

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

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

import java.nio.file.Files;
import java.util.*;

class CaesarCipherApp {
Copy link
Owner

Choose a reason for hiding this comment

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

Когда в Java приложении всего два файла это всегда ошибка. Разбиение на большое количество классов делается не для того чтобы все выглядело весомо, а для того чтобы код проще обслуживался и легче делился между членами команды. Поэтому хоть маленькая хоть большое приложение стоит сразу привыкнуть писать в корпоративном стандарте


switch (choice) {
case "1":
encryptMode(scanner);
Copy link
Owner

Choose a reason for hiding this comment

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

правильнее было бы сначала собрать все данные а потом вызывать операцию по их обработке

if (inputFile == null) return;

int key = getKey(scanner);
if (key == -1) return;
Copy link
Owner

Choose a reason for hiding this comment

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

тут какая-то магия, почему -1 а не -2 или -100

* Расшифровка файла по ключу.
*
* @param scanner пользователь выбирает ключ
* @getInputFile запрашиваем у пользователя путь к файлу
Copy link
Owner

Choose a reason for hiding this comment

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

за то что есть документация однозначно плюс, но ещё лучше чтобы она была не на русском языке а на английском

* иначе возвращает null.
*/

private File getInputFile(Scanner scanner) {
Copy link
Owner

Choose a reason for hiding this comment

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

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

key = Integer.parseInt(input);
if (key < 1 || key > maxKey) {
System.out.println("Ключ вне допустимого диапазона.");
key = -1;
Copy link
Owner

Choose a reason for hiding this comment

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

вот это число -1 лучше вынести в константу и как-то осмысленно её назвать

* @param scanner используется для получения ввода от пользователя (путь к файлу, выбор ключа).
*/

private void statisticalAnalysisMode(Scanner scanner) {
Copy link
Owner

Choose a reason for hiding this comment

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

на первый взгляд кажется правильным спрашивать пользователя внутри метода, но если задуматься о том что это приложение будет превращаться в мобильные, или в веб приложение, или в Desktop ную версию, то во всех этих случаях придётся переписывать всё и везде а хотелось бы переписать только места где вводятся данные. Достаточно представить себе что предложение должно работать так как работает любой сайт. И написание в таком стиле обслуживается в дальнейшем намного проще

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