Uwaga: Próbka kodu jest napisana w języku c #, ale to nie powinno mieć znaczenia. Umieściłem c # jako tag, ponieważ nie mogę znaleźć bardziej odpowiedniego. Chodzi o strukturę kodu.
Czytam Clean Code i staram się zostać lepszym programistą.
Często staram się przestrzegać zasady pojedynczej odpowiedzialności (klasy i funkcje powinny robić tylko jedną rzecz), szczególnie w funkcjach. Może moim problemem jest to, że „jedna rzecz” nie jest dobrze zdefiniowana, ale nadal…
Przykład: Mam listę puchu w bazie danych. Nie obchodzi nas, czym jest Puszysty. Chcę, żeby klasa odzyskała puszyste. Jednak puchary mogą się zmieniać zgodnie z pewną logiką. W zależności od logiki ta klasa zwróci dane z pamięci podręcznej lub pobierze najnowsze dane z bazy danych. Można powiedzieć, że zarządza puszystymi, i to jedno. Aby to uprościć, załóżmy, że załadowane dane są dobre przez godzinę, a następnie należy je ponownie załadować.
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()
wydaje mi się w porządku. Użytkownik prosi o niektóre puszyste, zapewniamy je. W razie potrzeby odzyskanie ich od DB, ale można to uznać za część zdobywania puchu (to oczywiście nieco subiektywne).
NeedsReload()
wydaje się również słuszne. Sprawdza, czy musimy ponownie załadować puchaty. UpdateNextLoad jest w porządku. Aktualizuje czas następnego przeładowania. to zdecydowanie jedna rzecz.
Czuję jednak, że tego, LoadFluffies()
czego nie można opisać jako jednej rzeczy. Pobiera dane z bazy danych i planuje następne przeładowanie. Trudno argumentować, że obliczenie czasu do następnego przeładowania jest częścią pozyskiwania danych. Nie mogę jednak znaleźć lepszego sposobu na zrobienie tego (zmiana nazwy funkcji LoadFluffiesAndScheduleNextLoad
może być lepsza, ale sprawia to, że problem jest bardziej oczywisty).
Czy istnieje eleganckie rozwiązanie, aby naprawdę napisać tę klasę według SRP? Czy jestem zbyt pedantyczny?
A może moja klasa tak naprawdę nie robi tylko jednej rzeczy?
DateTime.UtcNow
aby uniknąć zmiany czasu na letni lub nawet zmiany bieżącej strefy czasowej.Odpowiedzi:
Jeśli ta klasa naprawdę była tak trywialna, jak się wydaje, to nie trzeba się martwić o naruszenie SRP. A co jeśli funkcja 3-liniowa ma 2 linie robiące jedną rzecz, a inna 1 linia robi inną rzecz? Tak, ta trywialna funkcja narusza SRP i co z tego? Kogo to obchodzi? Naruszenie SRP staje się problemem, gdy sprawy stają się bardziej skomplikowane.
Twój problem w tym konkretnym przypadku najprawdopodobniej wynika z faktu, że klasa jest bardziej skomplikowana niż kilka linii, które nam pokazałeś.
W szczególności problem najprawdopodobniej polega na tym, że ta klasa nie tylko zarządza pamięcią podręczną, ale prawdopodobnie także zawiera implementację
GetFluffiesFromDb()
metody. Naruszenie SRP ma miejsce w klasie, a nie w kilku trywialnych metodach pokazanych w opublikowanym kodzie.Oto sugestia, jak radzić sobie z wszelkiego rodzaju przypadkami, które należą do tej ogólnej kategorii, za pomocą Wzoru Dekoratora .
i jest używany w następujący sposób:
Zauważ, że
CachingFluffiesProvider.GetFluffies()
nie boję się zawrzeć kodu, który sprawdza czas i aktualizuje, ponieważ to trywialne rzeczy. Mechanizm ten zajmuje się i radzi sobie z SRP na poziomie projektowania systemu, tam gdzie ma to znaczenie, a nie na poziomie drobnych indywidualnych metod, gdzie i tak nie ma to znaczenia.źródło
Sama klasa wydaje mi się w porządku, ale masz rację, że
LoadFluffies()
nie dokładnie to, co reklamuje nazwa. Jednym prostym rozwiązaniem byłaby zmiana nazwy i przeniesienie jawnego przeładowywania z GetFluffies do funkcji z odpowiednim opisem. Coś jakwygląda mi na czystą (również dlatego, że jak mówi Patrick: składa się z innych drobnych funkcji posłusznych SRP), a szczególnie wyraźnie, co jest czasami równie ważne.
źródło
Wierzę, że twoja klasa robi jedną rzecz; to pamięć podręczna z limitem czasu. LoadFluffies wydaje się bezużyteczną abstrakcją, chyba że wywołasz ją z wielu miejsc. Myślę, że lepiej byłoby pobrać dwie linie z LoadFluffies i umieścić je w warunku NeedsReload w GetFluffies. Uczyniłoby to implementację GetFluffies o wiele bardziej oczywistą i nadal jest czystym kodem, ponieważ komponujesz pojedyncze podprogramy odpowiedzialności, aby osiągnąć jeden cel, czyli buforowane pobieranie danych z bazy danych. Poniżej znajduje się zaktualizowana metoda zdobywania puchu.
źródło
Twoje instynkty są prawidłowe. Twoja klasa, choć niewielka, robi za dużo. Powinieneś podzielić logikę buforowania odświeżania czasowego na całkowicie ogólną klasę. Następnie utwórz specyficzną instancję tej klasy do zarządzania Fluffies, coś takiego: (nieskompilowany, działający kod pozostanie jako ćwiczenie dla czytelnika):
Dodatkową zaletą jest to, że teraz bardzo łatwo można przetestować TimedRefreshCache.
źródło
Twoja klasa jest w porządku, SRP dotyczy klasy, a nie funkcji, cała klasa jest odpowiedzialna za dostarczanie „Fluffies” z „Source Data”, więc jesteś wolny w wewnętrznej implementacji.
Jeśli chcesz rozwinąć mechanizm cahing, możesz stworzyć klasę odpowiedzialną za oglądanie źródła danych
źródło