Как правильно выстроить обработку исключений и насколько соблюдается SOLID? Оцените код C#
Проект совсем небольшой, хотел бы получить критику по поводу кода, это моё решённое тестовое задание. Его уже проверили, поэтому делаю рефакторинг и работаю над ошибками уже для себя. Была проведена работа по соответствию принципам SOLID, поэтому буду рад критике по этому поводу - чего упустил, чего сделал неправильно. Обработка исключений пожалуй самая большая проблема, множественные try catch замедляют программу, поэтому я оставил всего один. Если не затруднит дайте пожалуйста рекомендации по поводу работы с исключениями в программе или посоветовали какой-нибудь материал. И в конце концов прошу оценить код в целом, возможно с точки зрения производительности или потенциальных проблем. Вот ссылка на весь код: https://github.com/deflarten/SaveAndPrintProject
Ответы (1 шт):
Чтож, пойдем с начала
- Нет юнит тестов
- Проект вперемешку с солюшеном
- Что за папка "Interfaces"? А где папки "CLasses" и "Enums"?
- Каша с названиями
Какие то несуразные названия
ICsvConstructor
зачем для этого нужен интерфейс? Интерфейс нужен, если планируется иметь несколько реализаций. Например, если вы планируете и в CVS и в Word - тогда можно интерфейс.
И почему "Constructor"? Откуда это слово вообще? Для экспорта данных есть слово - Export.
IDbProvider
Provide - это "предоставлять", то есть TimeProvider - предоставляем время. DbProvider - предоставляет БД. В методах интерфейса нет ничего про БД
IWebProvider public string GetDownloadedFilename(string url);
Во-первых, это не предоставляет Web.
Во-вторых - название GetDownloadedFilename предполагает, что файл уже скачан и надо только его имя вернуть.
IYmlParser public IEnumerable GetItems(string filename, string shopId);
Почему бы парсеру не иметь метод Parse? Что такое shopId? Что за Items?
class Item
Почему бы просто не назвать class MyClass? Названия классов и интерфейсов должны выражать их назначение, а не быть случайным набором букв.
PrintExecutor
Почему для других классов есть интерфейс, а для этого - нет? Также Print и Execute - это 2 действия, так? Класс походу не делает ни того, ни другого.
- С одной стороны у вас какое то подобие IoC
private static ServiceProvider ConfigureServices() {...}
С другой стороны
var printExecutor = new PrintExecutor(dbProvider, csvConstructor);
Дальше читать не стал.
По итогу:
Видно, что вы начинающий, который старался как мог.
Хорошо: пытаетесь прийти к какой то архитектуре
Плохо:
- проблемы с именованием типов и неймспейсов
- непонятные методы типа
GetString(int foo, int bar)- название и сигнатура метода должны говорить о том, что он делает - Проблемы с пониманием, когда надо интерфейсы и когда не надо и зачем вообще интерфейсы нужны
- Не следуете своей же архитектуре - вроде IoC, а вроде и не IoC
- Пробелы в самой архитектуре. У вас в задании сказано, что вы обрабатываете команды, но нет класса "Команда" у вас. С одной стороны используете интерфейсы для (наверное) гибкости и расширяемости, с другой стороны обработка команд у вас простой swith-case