Руки бы оторвать за три класса в файле!
Это мне еще повезло, что ИДЕЯ их вот так отображает:
Не стоит импортировать с *, чуть медленнее запускается и со временем запутаешься в зависимостях.
Статическая коллекция, если ее можно сделать не статической, и при этом ничего не сломать -- не нужна.
Можно считать ее как утечку памяти, ведь она постоянно проинициализирована и хранит в себе данные...
Лучше не вызывать лишний раз toString() -- сбережешься от лишнего NPE, ведь оно само приводится к строке при конкатенации.
В твоем случае получишь ексепшен, в случае без toString() строку null.
У тебя:
public static String getRecordsAsString() {
String result = "";
for (String record : commonLog)
result += record + "\n";
return result;
}Было бы быстрее:
public static String getRecordsAsString() {
StringBuilder result = new StringBuilder();
for (String record : commonLog)
result.append(record).append("\n");
return result.toString();
}При конкатенации строк каждый раз создается инстанс StringBuilder, в моем коде он создан только один раз.
Правда, компилятор может и соптимизировать подобные вещи, но лучше перестраховаться.
Вместо (float) 10.0 можно написать 10f. Там еще с другими типами такая штука есть.
Игнорируешь результат delete(). Он возвращает статус (из доков):
* @return <code>true</code> if and only if the file or directory is
* successfully deleted; <code>false</code> otherwiseЕсли файл не удалится, дальше все пойдет по пизде и будет глючить, судя по коду.
Строка 106 (№1) Files.write(Paths.get("jphon.config"), Arrays.asList(config), Charset.forName("US-ASCII"), StandardOpenOption.CREATE_NEW);
ИДЕЯ говорит мне, что если у тебя один элемент, то вместо Arrays.asList(obj) лучше писать Collections.singletonList(obj).
Но учти -- этот массив будет иммутабельным.
Строка 106 (№2) Files.write(Paths.get("jphon.config"), Arrays.asList(config), Charset.forName("US-ASCII"), StandardOpenOption.CREATE_NEW);
Чому US-ASCII? По стандарту JSON должен лежать в одном из: UTF-{8,16,32}.
А интерфейс здесь вообще нужен? Просто я ща поискал по коду, он используется только один раз -- после его объявления.
Если у тебя метод бросает Exception, то он уже бросает IOException, так как это сабкласс
И да, просто на будущее: если ты можешь обработать исключение сразу же, то сделай это. Чем выше по стеку вызовов оно проберется, тем сложнее его отладить.
Поле легко можно преобразовать в локальную переменную, больше нигде не используется.
Лучше сабкласнуть Exception своим классом. Так и семантика лучше, и какие-нибудь данные кроме строки с информацией туда можешь запихнуть.
} catch (IOException|SecurityException e) {
throw new Exception("Ошибка при запуске процесса!");
}За такое отрывают руки -- ты не залоггировал причину на месте или не пробросил source exception (ексепшн, который ты поймал) вверх по стеку.
Экспресс-фикс: throw new Exception("Ошибка при запуске процесса!", e);, ну лучше, как я уже говорил, сабкласснуть Exception.
Думаю, если coreProcess не равен null, то надо еще проверить isAlive().
Ух, ебать! Хуй разберешься, что здесь происходит, как-то все в одном бедном конструкторе навалено.
Разнеси по разным методам, даже если они будут вызываться из одного места. Итак...
Nested try. Здесь тот самый случай, когда ексепшн лучше пробросить наверх, а то совсем каша.
Не понял, что здесь происходит -- ты считываешь ЖСОН по строчке? У него же структура есть... Хотя если это работает, то я упоролся.
Использование результат присвоения немного странно, начинаешь подозревать, что там должно было быть ==.
На будущее -- строки в switch сравнивать небезопасно из-за коллизии хешей, здесь они слабенькие.
break; в последнем выражении свитча, конечно, не обязателен, но лучше поставить -- а вдруг ты после этого еще один кейс допишешь?
Охуеешь отлаживать.
Строка 95 CommonLog.addRecord("! Json нихера не парсится. Что-то не так пошло. Проблемная строка:");
Ты залоггировал JSON, что не распарсился, но не записал сам ексепшен, а там тоже есть полезные сведения.
Тут сразу в начале файла четыре поля, которые можно преобразовать в локальные переменные, но не суть.
Именно в этом файле могу ошибаться, так как со свингом не работал.
trayMenuShow.addActionListener((ActionEvent e) -> {
toggleVisibility(aboutFrame);
});можно заменить на
trayMenuShow.addActionListener((ActionEvent e) -> toggleVisibility(aboutFrame)); if (getState() == Frame.NORMAL) {
setState(Frame.ICONIFIED);
} else {
setState(Frame.NORMAL);
}можно полностью заменить на тернарный оператор setState(getState() == Frame.NORMAL ? Frame.ICONIFIED : Frame.NORMAL);, но я их не люблю.
Runtime.getRuntime().addShutdownHook(new Thread(() -> {
coreHandler.stopProcess();
}));можно заменить на Runtime.getRuntime().addShutdownHook(new Thread(() -> coreHandler.stopProcess()));, но еще лучше заменить на Runtime.getRuntime().addShutdownHook(new Thread(coreHandler::stopProcess));.
CoreParser coreParser;
coreParser = new CoreParser(is);Поскольку эта переменная присваивается, но не считывается, компилятор имеет полное право это выражения удалить, емнип. Оно здесь нужно?