Skip to content

Fix formatting in Program.cs#32

Open
Sane4ka126 wants to merge 1 commit into
masterfrom
Sane4ka126-patch-21
Open

Fix formatting in Program.cs#32
Sane4ka126 wants to merge 1 commit into
masterfrom
Sane4ka126-patch-21

Conversation

@Sane4ka126
Copy link
Copy Markdown
Owner

Lab 6 - Safe Refactoring with Unit Tests Coverage
image
image

Виконати безпечний рефакторинг коду EchoServer з повним покриттям юніт-тестами для покращення:

  • Тестованості - можливість покрити код автоматичними тестами
  • Підтримуваності - легкість внесення змін та виправлення помилок
  • Розширюваності - можливість додавати нову функціональність
  • Дотримання SOLID принципів** - якісна архітектура коду
    Проблеми для тестування: 1. Жорстка залежність від мережі - TcpListener, TcpClient, UdpClient неможливо замокати 2. Статичний метод Main - складно тестувати точку входу 3. Відсутність абстракцій - прямі виклики Console.WriteLine 4. Змішана відповідальність - Main запускає і сервер, і UDP sender 5. Відсутність інтерфейсів - неможливо підмінити залежності 6. Відсутність Dependency Injection
    Проблема:
    У вихідному коді використовуються системні класи TcpListener, TcpClient, NetworkStream, UdpClient. Всі ці класи позначені як sealed в .NET, що означає неможливість їх успадкування. Фреймворки для мокування (Moq, NSubstitute) працюють через динамічне створення підкласів, тому sealed класи неможливо замокати. Це робить автоматизоване тестування повністю неможливим.
    Наслідки:

Не можна написати жодного юніт-тесту
Неможливо перевірити логіку без реального мережевого підключення
Кожен тест вимагав би запуску справжнього сервера на реальному порту
Тести були б повільними (мережеві операції)
Тести були б нестабільними (залежність від мережі)
Жорстка залежність від Console (логування)
Проблема: По всьому коду розкидані прямі виклики Console.WriteLine() для виведення інформації. Console - це статичний клас, який неможливо замокати. У тестах немає способу перевірити, чи виводяться правильні повідомлення, чи взагалі щось виводиться.
Приклади з коду:
• Console.WriteLine($"Server started on port {_port}.");
• Console.WriteLine("Client connected.");
• Console.WriteLine($"Echoed {bytesRead} bytes to the client.");
• Console.WriteLine($"Error: {ex.Message}");
Наслідки:
• Неможливо перевірити, що логується при помилках
• Неможливо перевірити, що виводяться правильні повідомлення
• Неможливо підмінити логування на файл або інший механізм
• Порушується принцип Dependency Inversion - залежність від конкретної реалізації

Порушення Single Responsibility Principle
Проблема: Клас EchoServer виконує одночасно 5 різних відповідальностей:

  1. Управління життєвим циклом сервера - метод StartAsync() запускає listener
  2. Прийняття TCP з'єднань - AcceptTcpClientAsync()
  3. Обробка клієнтських з'єднань - метод HandleClientAsync()
  4. Читання та запис мережевих даних - робота з NetworkStream
  5. Логування всіх подій - виклики Console.WriteLine()
    Додатково в Main методі змішано:
    • Запуск echo-сервера
    • Запуск UDP sender
    • Обробка користувацького вводу
    • Управління всім життєвим циклом додатку
    Наслідки:
    • Складно зрозуміти, що робить клас
    • Складно змінювати одну частину без ризику зламати іншу
    • Неможливо повторно використати частини логіки
    • Високий Cyclomatic Complexity (15+)
    • Низький Maintainability Index (60)
    Відсутність абстракцій та інтерфейсів
    Проблема: В коді немає жодного інтерфейсу. Всі залежності - це конкретні класи:
    • TcpListener _listener - конкретний клас
    • TcpClient client - конкретний клас
    • NetworkStream stream - конкретний клас
    • UdpClient _udpClient - конкретний клас
    Це означає "жорстке зчеплення" (tight coupling) - кожен клас намертво прив'язаний до конкретних реалізацій.
    Наслідки:
    • Неможливо підмінити залежності на mock-об'єкти
    • Неможливо створити альтернативні реалізації
    • Порушується Dependency Inversion Principle
    • Порушується Open/Closed Principle
    • Код негнучкий та нерозширюваний

StartAsync(): _listener = new TcpListener(IPAddress.Any, _port); // створення всередині HandleClientAsync(): using (NetworkStream stream = client.GetStream()) // отримання всередині UdpTimedSender constructor: _udpClient = new UdpClient(); // створення всередині

Відсутність валідації параметрів
Проблема: Конструктори та методи не перевіряють вхідні параметри:
public EchoServer(int port)
{
_port = port; // А що якщо port = -1? Або 99999?
}

private async Task HandleClientAsync(TcpClient client, ...)
{
// А що якщо client == null?
using (NetworkStream stream = client.GetStream()) // NullReferenceException!
}
Наслідки:
• Можливі NullReferenceException у runtime
• Можливі ArgumentException з невалідними значеннями
• Проблеми виявляються пізно - коли метод виконується
• Порушується Fail-Fast принцип
• Складно дебажити - помилка може проявитися далеко від джерела
Монолітна структура файлів
Проблема: Весь код розміщений в одному файлі Program.cs розміром 172 рядки. В одному файлі змішано:
• Клас EchoServer (основна логіка)
• Клас UdpTimedSender (окрема функціональність)
• Метод Main (точка входу)
• Вся бізнес-логіка
• Вся інфраструктура
Наслідки:
• Важко навігуватися по коду
• Складно знайти потрібну функціональність
• Порушення принципу організації коду
• Неможливість повторного використання окремих частин
• Складно працювати в команді (merge conflicts)
• Важко розуміти структуру проекту
Code Quality проблеми (Sonar warnings)
Проблема: SonarQube виявив 9 critical/major попереджень:

  1. CS8618: Non-nullable поле _timer не ініціалізується в конструкторі
  2. CS8618: Non-nullable поле _listener не ініціалізується в конструкторі
  3. S2933: Поле _cancellationTokenSource може бути readonly
  4. S3881: Неправильний IDisposable pattern - відсутній GC.SuppressFinalize()
  5. S1118: Клас Program має приватний конструктор або має бути static
  6. CS1998: Async метод Main не містить await
  7. S125: Закоментований код (якщо був)
  8. S107: Занадто багато параметрів в методах
  9. S1066: Складні умови можна спростити
    Наслідки:
    • Потенційні NullReferenceException
    • Витоки пам'яті через неправильний Dispose
    • Непередбачувана поведінка
    • Складність підтримки коду
    • Технічний борг
    Високі метрики складності
    Проблема: Метрики якості коду показували погані значення:
    • Cyclomatic Complexity: 15+ (норма: до 10)
    o Багато вкладених if/try/while конструкцій
    o Складна логіка в методах
    • Maintainability Index: 60 (норма: 80+)
    o Код важко підтримувати
    o Високий ризик внесення помилок при змінах
    • Lines of Code per method: 30-50 (норма: до 20)
    o Методи занадто великі
    o Змішана логіка різних рівнів абстракції
    Наслідки:
    • Код складний для розуміння
    • Високий ризик помилок при змінах
    • Важко знаходити баги
    • Довгий час на онбординг нових розробників
    Відсутність розділення concerns
    Проблема: В методі HandleClientAsync змішано три різні рівні абстракції:
  10. Низький рівень: робота з байтами, буферами (byte[] buffer = new byte[8192])
  11. Середній рівень: мережеві операції (stream.ReadAsync, stream.WriteAsync)
  12. Високий рівень: бізнес-логіка (echo функціональність)
  13. Інфраструктура: обробка помилок, логування
    Все це в одному методі на 30+ рядків.
    Наслідки:
    • Неможливо протестувати бізнес-логіку окремо від мережі
    • Неможливо змінити один рівень без впливу на інші
    • Порушується Separation of Concerns принцип
    • Код важко читати та розуміти
    Що було додано для покращення
    Створили систему абстракцій (10 файлів)
    Рішення: Обгорнули всі sealed класи в інтерфейси та wrapper-класи:
    Файл ILogger.cs:
    • Інтерфейс з методами Log(string) та LogError(string)
    • Відокремлює логування від бізнес-логіки
    • Можна замокати в тестах
image Файл ConsoleLogger.cs: image

Реалізація ILogger через Console.WriteLine
Використовується в production коді
Легко замінити на FileLogger або DatabaseLogger

Файл INetworkStreamWrapper.cs:
image

Інтерфейс-обгортка для NetworkStream
Методи ReadAsync та WriteAsync
Успадковує IDisposable для правильного cleanup

Файл NetworkStreamWrapper.cs:
image

Wrapper клас навколо реального NetworkStream
Делегує виклики до оригінального об'єкту
Можна замокати в тестах

Файл ITcpClientWrapper.cs:
image

Інтерфейс-обгортка для TcpClient
Метод GetStream() повертає INetworkStreamWrapper
Метод Close() для закриття з'єднання

Файл TcpClientWrapper.cs:
image

Wrapper навколо реального TcpClient
Створює NetworkStreamWrapper при виклику GetStream()

Файл ITcpListenerWrapper.cs:

Інтерфейс-обгортка для TcpListener
Методи Start(), Stop(), AcceptTcpClientAsync()
Повертає ITcpClientWrapper замість TcpClient

Файл TcpListenerWrapper.cs:
image

Wrapper навколо реального TcpListener
Перетворює TcpClient в TcpClientWrapper

Файл ITcpListenerFactory.cs:
image

Factory інтерфейс для створення listener
Метод Create(IPAddress, int) повертає ITcpListenerWrapper
Дозволяє в тестах повернути mock-listener

Файл TcpListenerFactory.cs:
image

Реалізація factory для production
Створює справжній TcpListenerWrapper

Результат:

Всі sealed класи тепер можна мокати
Всі мережеві операції контролюються через інтерфейси
Тести можуть підмінити будь-яку залежність
Дотримано Dependency Inversion Principle
Впровадили Dependency Injection
Рішення: Всі залежності тепер передаються через конструктор:
Конструктор приймає: - int port - порт сервера - ILogger logger - абстракція логування - ITcpListenerFactory listenerFactory - фабрика listener Всі параметри зберігаються в readonly полях Всі параметри валідуються на null
Що це дає:
• Клас не створює залежності сам - отримує ззовні
• В тестах передаємо mock-об'єкти
• В production передаємо справжні реалізації
• Loose coupling замість tight coupling
• Можна легко змінити реалізацію без зміни коду класу
UdpTimedSender.cs:
plaintext
Конструктор приймає:

  • string host - адреса для відправки
  • int port - порт для відправки
  • ILogger logger - абстракція логування
image image

UdpClient створюється всередині (поки), але логер - через DI
Покращення:
• Можна перевірити в тестах, що правильні повідомлення логуються
• Можна замінити логування без зміни класу
Роль: "Composition Root" - місце де збираємо всі залежності Створюємо: 1. ConsoleLogger logger = new ConsoleLogger(); 2. ITcpListenerFactory factory = new TcpListenerFactory(); 3. EchoServerService server = new EchoServerService(5000, logger, factory); 4. UdpTimedSender sender = new UdpTimedSender(host, port, logger); Передаємо все готове в сервіси
Розділили відповідальності (SRP)
Рішення: Розбили монолітний клас на окремі компоненти:
Файл EchoServerService.cs:

image image image

Відповідальність: Тільки обробка TCP клієнтів
• Метод StartAsync() - запускає listener і приймає з'єднання
• Метод HandleClientAsync() - обробляє одного клієнта
• Використовує DI для всіх залежностей
• Не знає про UDP, Console, або інші сервіси

Що покращилось:
• Чітка, зрозуміла відповідальність
• Легко читати та розуміти код
• Cyclomatic Complexity знизився з 15+ до 3-5
• Кожен метод робить одну річ
Файл UdpTimedSender.cs:
Відповідальність: Тільки відправка UDP пакетів по таймеру
• Метод StartSending() - запускає таймер
• Метод StopSending() - зупиняє таймер
• Callback метод - відправляє один пакет
• Не знає про TCP сервер або інші компоненти
Що покращилось:
• Можна використати окремо від echo-сервера
• Можна протестувати ізольовано
• Зрозуміло, що клас робить
Файл ConsoleLogger.cs:
Відповідальність: Тільки виведення в консоль
• Метод Log() - виводить інформаційні повідомлення
• Метод LogError() - виводить помилки з кольором
Що покращилось:
• Централізоване логування
• Легко замінити на інший механізм (файл, БД, Serilog)
• Можна додати фільтрацію, форматування в одному місці
Файл Program.cs:
Відповідальність: Тільки точка входу та ініціалізація
• Створення залежностей
• Запуск сервісів
• Обробка користувацького вводу (натискання 'q')
• Graceful shutdown

image

@sonarqubecloud
Copy link
Copy Markdown

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.

1 participant