Примечание. Пример кода написан на c #, но это не должно иметь значения. Я поставил c # в качестве тега, потому что не могу найти более подходящий. Это о структуре кода.
Я читаю Чистый код и пытаюсь стать лучшим программистом.
Я часто сталкиваюсь с трудностями следования принципу единой ответственности (классы и функции должны выполнять только одно), особенно в функциях. Может быть, моя проблема в том, что «одна вещь» не очень четко определена, но все же ...
Пример: у меня есть список Пушистиков в базе данных. Нам не важно, что такое Пушистик. Я хочу класс, чтобы восстановить пушистики. Тем не менее, пушистые могут меняться в соответствии с некоторой логикой. В зависимости от логики, этот класс будет возвращать данные из кэша или получать последние данные из базы данных. Можно сказать, что он управляет пушистиками, и это одно. Для простоты, скажем, загруженные данные хороши в течение часа, а затем их необходимо перезагрузить.
class FluffiesManager
{
private Fluffies m_Cache;
private DateTime m_NextReload = DateTime.MinValue;
// ...
public Fluffies GetFluffies()
{
if (NeedsReload())
LoadFluffies();
return m_Cache;
}
private NeedsReload()
{
return (m_NextReload < DateTime.Now);
}
private void LoadFluffies()
{
GetFluffiesFromDb();
UpdateNextLoad();
}
private void UpdateNextLoad()
{
m_NextReload = DatTime.Now + TimeSpan.FromHours(1);
}
// ...
}
GetFluffies()
кажется, хорошо для меня. Пользователь просит несколько пушистиков, мы их предоставляем. Собираюсь восстановить их из БД, если это необходимо, но это можно считать частью получения пушистых копий (конечно, это несколько субъективно).
NeedsReload()
тоже кажется правильным. Проверяет, нужно ли перезагрузить пушистики. UpdateNextLoad в порядке. Обновляет время для следующей перезагрузки. это определенно одна вещь.
Однако я чувствую, что LoadFluffies()
нельзя описать как одну вещь. Он получает данные из базы данных и планирует следующую перезагрузку. Трудно утверждать, что вычисление времени для следующей перезагрузки является частью получения данных. Тем не менее, я не могу найти лучший способ сделать это (переименование функции LoadFluffiesAndScheduleNextLoad
может быть лучше, но это только делает проблему более очевидной).
Есть ли элегантное решение, чтобы действительно написать этот класс в соответствии с SRP? Я слишком педантичен?
Или, может быть, мой класс на самом деле не делает только одну вещь?
DateTime.UtcNow
чтобы избежать перехода на летнее время или даже изменения текущего часового пояса.Ответы:
Если бы этот класс был таким же тривиальным, как кажется, тогда не было бы необходимости беспокоиться о нарушении SRP. Так что, если 3-строчная функция имеет 2 строки, выполняющие одно, а еще 1 строку, выполняющую другое действие? Да, эта тривиальная функция нарушает ПСП, и что с того? Какая разница? Нарушение SRP становится проблемой, когда все становится сложнее.
Ваша проблема в данном конкретном случае, скорее всего, связана с тем, что класс более сложен, чем несколько строк, которые вы нам показали.
В частности, проблема, скорее всего, заключается в том, что этот класс не только управляет кэшем, но также, вероятно, содержит реализацию
GetFluffiesFromDb()
метода. Таким образом, нарушение SRP относится к классу, а не к нескольким тривиальным методам, показанным в опубликованном вами коде.Итак, вот предложение о том, как обрабатывать все виды дел, которые попадают в эту общую категорию, с помощью шаблона Decorator .
и он используется следующим образом:
Обратите внимание, как
CachingFluffiesProvider.GetFluffies()
не боится содержать код, который выполняет проверку и обновление времени, потому что это тривиальные вещи. Этот механизм выполняет и обрабатывает SRP на уровне проектирования системы, где это имеет значение, а не на уровне крошечных отдельных методов, где это все равно не имеет значения.источник
Сам твой класс кажется мне подходящим, но ты прав в том,
LoadFluffies()
что не совсем то, что имя рекламирует. Одним простым решением было бы изменить имя и переместить явную перезагрузку из GetFluffies в функцию с соответствующим описанием. Что-то вродевыглядит чистым для меня (также потому что, как говорит Патрик: он состоит из других крошечных функций, подчиняющихся SRP), и особенно ясным, что иногда так же важно.
источник
Я верю, что ваш класс делает одну вещь; это кэш данных с таймаутом. LoadFluffies кажется бесполезной абстракцией, если вы не вызываете ее из нескольких мест. Я думаю, что было бы лучше взять две строки из LoadFluffies и поместить их в условие NeedsReload в GetFluffies. Это сделало бы реализацию GetFluffies намного более очевидной и по-прежнему чистым кодом, поскольку вы составляете подпрограммы с одной ответственностью для достижения единой цели - кэшированного извлечения данных из базы данных. Ниже приведен обновленный метод get fluffies.
источник
Ваши инстинкты верны. Ваш класс, пусть и маленький, делает слишком много. Вы должны разделить логику кэширования обновления по времени в полностью общий класс. Затем создайте конкретный экземпляр этого класса для управления Fluffies, что-то вроде этого (не скомпилировано, рабочий код оставлен в качестве упражнения для читателя):
Дополнительным преимуществом является то, что теперь очень легко протестировать TimedRefreshCache.
источник
Ваш класс в порядке, SRP - это класс, а не функция, весь класс отвечает за предоставление «Пушистиков» из «Источника данных», поэтому вы свободны во внутренней реализации.
Если вы хотите расширить механизм кэширования, вы можете создать класс, отвечающий за просмотр источника данных.
источник