- Remove deprecated service interfaces (CooldownService, IgnoreService, PMService, PlayerDataService, SpyService) - Add new unified service interfaces (MessagingService, PlayerService, PunishmentService) - Extract FilterResult as standalone class from AdvancedMessageFilter - Implement individual filter classes (CharacterFilter, FloodFilter, SpamFilter, SwearFilter, UrlFilter) - Add moderation commands (BanCommand, WarnCommand, SilentWarnCommand, UnbanCommand) - Introduce data models for punishment tracking (BanRecord, WarnEntry) - Refactor messaging service with new MessagingServiceImpl implementation - Add mute service utilities (MuteDataStorage, MutePermissionChecker) - Implement PunishmentServiceImpl with snapshot tracking - Add Discord integration components (DiscordConfig, DiscordMessageService) - Introduce command handlers for gradient module (ColorCommandHandler, PrefixCommandHandler) - Add renderer components (EmojiComponent, GradientBuilder, UrlProcessor) - Add utility classes (ColorConverter, MessageSender, PlaceholderProcessor) - Update configuration files and plugin metadata - Add documentation files (CODE_REVIEW_RU.md, REFACTORING_SERVICES.md) - Consolidate service architecture for improved maintainability and separation of concerns
11 KiB
🔍 Code Review: Проблемы архитектуры (SOLID, DRY, KISS)
🔴 Критические проблемы
1. DRY: Дублирование кода в конфигах
Файлы: AppearanceConfig.java, MessagesConfig.java, FiltersConfig.java
Проблема:
- Каждый конфиг содержит 30-50 геттеров с одинаковой структурой
- ~300 строк повторяющегося кода
- При изменении логики нужно править в 3 местах
Решение:
// Создать базовый класс с generic методами
public abstract class BaseConfig {
protected String getString(String path, String def) { ... }
protected int getInt(String path, int def) { ... }
protected boolean getBoolean(String path, boolean def) { ... }
}
// Использовать в конфигах
public class MessagesConfig extends BaseConfig {
public String getNoPermission() {
return getString("general.no_permission", "&#CF6679Нет прав!");
}
}
2. SRP: ConfigManager делает слишком много
Файл: ConfigManager.java
Проблема:
- Управляет 5 конфигами
- Содержит логику обновления версий
- Имеет 30+ геттеров для настроек
- Нарушает Single Responsibility Principle
Решение:
// Разделить на отдельные классы
class ConfigManager {
private final AppearanceConfig appearance;
private final MessagesConfig messages;
// Только управление конфигами
}
class ConfigUpdater {
// Логика обновления версий
void updateConfig(FileConfiguration config) { ... }
}
3. KISS: ColorConverter неэффективен
Файл: ColorConverter.java
Проблема:
private static String convertAmpersandColors(String message) {
return message
.replaceAll("&0", "<black>")
.replaceAll("&1", "<dark_blue>")
// ... 16 вызовов replaceAll
}
- Каждый
replaceAllкомпилирует regex - O(n * 16) сложность
- Дублирование для
§и&
Решение:
private static final Map<String, String> COLOR_MAP = Map.ofEntries(
Map.entry("&0", "<black>"),
Map.entry("§0", "<black>"),
Map.entry("&1", "<dark_blue>"),
Map.entry("§1", "<dark_blue>")
// ...
);
private static String convertColors(String message) {
for (Map.Entry<String, String> entry : COLOR_MAP.entrySet()) {
message = message.replace(entry.getKey(), entry.getValue());
}
return message;
}
4. DRY: Дублирование проверок permissions
Файл: AdvancedMessageFilter.java
Проблема:
if (!player.hasPermission("lochat.bypass.swear") && !player.hasPermission("lochat.bypass.filter"))
if (!player.hasPermission("lochat.bypass.flood") && !player.hasPermission("lochat.bypass.filter"))
if (!player.hasPermission("lochat.bypass.spam") && !player.hasPermission("lochat.bypass.filter"))
Решение:
private boolean canBypassFilter(Player player, String filterType) {
return player.hasPermission("lochat.bypass." + filterType)
|| player.hasPermission("lochat.bypass.filter");
}
// Использование
if (!canBypassFilter(player, "swear")) {
FilterResult swear = swearFilter.filter(player, message);
// ...
}
5. OCP: EnhancedChatRenderer не расширяем
Файл: EnhancedChatRenderer.java
Проблема:
switch (placeholder) {
case "emoji" -> result = result.append(emoji);
case "prefix" -> result = result.append(prefix);
// Добавление нового плейсхолдера требует изменения кода
}
Решение:
// Strategy pattern для плейсхолдеров
interface PlaceholderResolver {
Component resolve(Player player, boolean isGlobal);
}
Map<String, PlaceholderResolver> resolvers = Map.of(
"emoji", (p, g) -> buildEmojiComponent(g),
"prefix", (p, g) -> buildPrefix(g),
"player", (p, g) -> buildPlayerComponent(p)
);
// Использование
Component resolved = resolvers.getOrDefault(placeholder,
(p, g) -> Component.text("{" + placeholder + "}"))
.resolve(player, isGlobal);
6. ISP: MuteServiceImpl не реализован полностью
Файл: MuteServiceImpl.java
Проблема:
@Override
public void save() {
// TODO: Implement save to file
}
private void load() {
// TODO: Implement load from file
}
Решение:
- Либо реализовать методы
- Либо разделить интерфейс на
MuteServiceиPersistentMuteService
7. Отсутствует зависимость VoiceChat
Файл: MuteServiceImpl.java
Проблема:
import de.maxhenkel.voicechat.api.BukkitVoicechatService; // ❌ Не найден
Решение:
// build.gradle.kts
dependencies {
compileOnly("de.maxhenkel.voicechat:voicechat-api:2.5.0")
}
Или сделать опциональной зависимостью:
private void initVoiceChat() {
try {
Class.forName("de.maxhenkel.voicechat.api.BukkitVoicechatService");
// Инициализация
} catch (ClassNotFoundException e) {
plugin.getLogger().info("VoiceChat not found, voice mute disabled");
}
}
8. SRP: ChatEventListener слишком сложный
Файл: ChatEventListener.java
Проблема:
- Метод
onChatсодержит 80+ строк - Обрабатывает: режим чата, фильтрацию, радиус, Discord, статистику
- Нарушает Single Responsibility
Решение:
class ChatEventListener {
private final ChatModeHandler modeHandler;
private final MessageFilterHandler filterHandler;
private final LocalChatHandler localHandler;
private final DiscordHandler discordHandler;
private final StatsHandler statsHandler;
@EventHandler
public void onChat(AsyncChatEvent event) {
Player sender = event.getPlayer();
String message = extractMessage(event);
// Делегируем обработку
ChatMode mode = modeHandler.determineMode(message);
if (!filterHandler.filter(sender, message)) {
event.setCancelled(true);
return;
}
if (mode == ChatMode.LOCAL) {
localHandler.handleLocal(event, sender);
}
discordHandler.sendToDiscord(sender, message, mode);
statsHandler.recordMessage(sender, mode);
}
}
9. DRY: Дублирование конвертации цветов
Файл: ColorConverter.java
Проблема:
private static String convertSectionColors(String message) { ... }
private static String convertAmpersandColors(String message) { ... }
Идентичная логика, только разные символы.
Решение:
private static String convertColors(String message, char symbol) {
return message
.replaceAll(symbol + "0", "<black>")
.replaceAll(symbol + "1", "<dark_blue>")
// ...
}
public static String convertLegacyFormats(String message) {
message = convertColors(message, '§');
message = convertColors(message, '&');
return message;
}
10. DIP: Прямая зависимость от LoChat
Файлы: EnhancedChatRenderer.java, ChatEventListener.java, и др.
Проблема:
com.loki.lochat.LoChat loChat = (com.loki.lochat.LoChat) plugin;
Нарушает Dependency Inversion Principle.
Решение:
// Создать интерфейс
interface ChatPlugin {
ConfigManager getConfigManager();
GradientModule getGradientModule();
DiscordIntegration getDiscordIntegration();
}
// LoChat реализует интерфейс
class LoChat extends JavaPlugin implements ChatPlugin { ... }
// Использование
class EnhancedChatRenderer {
private final ChatPlugin plugin;
public EnhancedChatRenderer(ChatPlugin plugin, boolean isGlobal) {
this.plugin = plugin;
// Нет каста
}
}
📊 Статистика проблем
| Принцип | Нарушений | Критичность |
|---|---|---|
| DRY | 4 | 🔴 Высокая |
| SRP | 3 | 🔴 Высокая |
| KISS | 2 | 🟡 Средняя |
| OCP | 1 | 🟡 Средняя |
| DIP | 1 | 🟡 Средняя |
| ISP | 1 | 🟢 Низкая |
✅ Что сделано хорошо
- ServiceRegistry - правильная реализация DI
- Разделение на модули - gradient, integrations, core
- Использование интерфейсов - api.service.*
- PluginInitializer/PluginShutdown - разделение логики инициализации
- BaseConfig - базовый класс для конфигов (уже есть!)
🎯 Приоритеты исправления
Высокий приоритет
- Исправить отсутствующую зависимость VoiceChat
- Упростить ColorConverter (KISS)
- Вынести проверки permissions в отдельный метод (DRY)
Средний приоритет
- Разделить ChatEventListener на handler'ы (SRP)
- Убрать касты к LoChat через интерфейс (DIP)
- Сделать EnhancedChatRenderer расширяемым (OCP)
Низкий приоритет
- Реализовать save/load в MuteServiceImpl (ISP)
- Упростить ConfigManager (SRP)
- Объединить методы конвертации цветов (DRY)
📝 Итого
Проект имеет хорошую базовую архитектуру (ServiceRegistry, модули, интерфейсы), но есть проблемы с:
- Дублированием кода (DRY) - особенно в конфигах и фильтрах
- Слишком большими классами (SRP) - ConfigManager, ChatEventListener
- Неэффективными алгоритмами (KISS) - ColorConverter
Рекомендую начать с исправления критических проблем (VoiceChat, ColorConverter, permissions), затем постепенно рефакторить остальное.