Skip to content

RomanovFadi | 4 tasks complete #31

Open
Fadeyzlodey wants to merge 1 commit intodemologin:mainfrom
Fadeyzlodey:main
Open

RomanovFadi | 4 tasks complete #31
Fadeyzlodey wants to merge 1 commit intodemologin:mainfrom
Fadeyzlodey:main

Conversation

@Fadeyzlodey
Copy link

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 class Main {
public static void main(String[] args) {
new MenuService().start();
Copy link

Choose a reason for hiding this comment

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

То что создание приложения и его старт это два разных метода – это хорошо, однако стоило бы назвать объект верхнего уровня как-то логично например application и передать в него компоненты нужные для его работы


public class BruteForce {

public static String bruteForceReport(String cipher) {
Copy link

Choose a reason for hiding this comment

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

Тут всё хорошо, но полезно привыкнуть делать методы объектными а не статическими


public class FrequencyAnalyzer {

private static final double[] EN_FREQ = {
Copy link

Choose a reason for hiding this comment

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

Технически это массив, поэтому константное имя лучше не давать

2.81,4.73,5.47,6.26,2.62,0.26,0.97,0.48,1.44,0.73,0.36,0.04,1.90,1.74,0.32,0.64,2.01
};

public static double chiSquared(String text, Language lang) {
Copy link

Choose a reason for hiding this comment

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

Тут однозначно плюс

public static int bestShift(String cipher, Language lang) {
int alphaLen = (lang == Language.EN ? CaesarCipher.getEnUp().length() : CaesarCipher.getRuUp().length());
double bestScore = Double.POSITIVE_INFINITY;
int bestK = 0;
Copy link

Choose a reason for hiding this comment

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

Неудачное имя переменной

double bestScore = Double.POSITIVE_INFINITY;
int bestK = 0;
for (int i = 0; i < alphaLen; i++) {
String cand = CaesarCipher.transform(cipher, i, false);
Copy link

Choose a reason for hiding this comment

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

В языке Java принято писать английские слова полностью даже если строка получается длинной

}

private static boolean isInAlphabet(char c, String alphabet) {
return alphabet.indexOf(c) >= 0;
Copy link

Choose a reason for hiding this comment

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

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

return alphabet.charAt(n);
}

public static String getEnUp() {
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