Jak uniknąć naruszenia SRP w klasie w celu zarządzania buforowaniem?

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 LoadFluffiesAndScheduleNextLoadmoż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?

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 .

/// Provides Fluffies.
interface FluffiesProvider
{
    Fluffies GetFluffies();
}

/// Implements FluffiesProvider using a database.
class DatabaseFluffiesProvider : FluffiesProvider
{
    public override Fluffies GetFluffies()
    {
        ... load fluffies from DB ...
        (the entire implementation of "GetFluffiesFromDb()" goes here.)
    }
}

/// Decorates FluffiesProvider to add caching.
class CachingFluffiesProvider : FluffiesProvider
{
    private FluffiesProvider decoree;
    private DateTime m_NextReload = DateTime.MinValue;
    private Fluffies m_Cache;

    public CachingFluffiesProvider( FluffiesProvider decoree )
    {
        Assert( decoree != null );
        this.decoree = decoree;
    }

    public override Fluffies GetFluffies()
    {
        if( DateTime.Now >= m_NextReload ) 
        {
             m_Cache = decoree.GetFluffies();
             m_NextReload = DatTime.Now + TimeSpan.FromHours(1);
        }
        return m_Cache;
    }
}

i jest używany w następujący sposób:

FluffiesProvider provider = new DatabaseFluffiesProvider();
provider = new CachingFluffiesProvider( provider );
...go ahead and use provider...

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.

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ś jak

public Fluffies GetFluffies()
{
  MakeSureTheFluffyCacheIsUpToDate();
  return m_Cache;
}

private void MakeSureTheFluffyCacheIsUpToDate()
{
  if( !NeedsReload )
    return;
  GetFluffiesFromDb();
  SetNextReloadTime();
}

wyglą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.

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.

public Fluffies GetFluffies()
{
    if (NeedsReload()) {
        GetFluffiesFromDb();
        UpdateNextLoad();
    }

    return m_Cache;
}

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):

public class TimedRefreshCache<T> {
    T m_Value;
    DateTime m_NextLoadTime;
    Func<T> m_producer();
    public CacheManager(Func<T> T producer, Interval timeBetweenLoads) {
          m_nextLoadTime = INFINITE_PAST;
          m_producer = producer;
    }
    public T Value {
        get {
            if (m_NextLoadTime < DateTime.Now) {
                m_Value = m_Producer();
                m_NextLoadTime = ...;
            }
            return m_Value;
        }
    }
}

public class FluffyCache {
    private TimedRefreshCache m_Cache 
        = new TimedRefreshCache<Fluffy>(GetFluffiesFromDb, interval);
    private Fluffy GetFluffiesFromDb() { ... }
    public Fluffy Value { get { return m_Cache.Value; } }
}

Dodatkową zaletą jest to, że teraz bardzo łatwo można przetestować TimedRefreshCache.

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

public class ModelWatcher
{

    private static Dictionary<Type, DateTime> LastUpdate;

    public static bool IsUpToDate(Type entityType, DateTime lastRead) {
        if (LastUpdate.ContainsKey(entityType)) {
            return lastRead >= LastUpdate[entityType];
        }
        return true;
    }

    //call this method whenever insert/update changed to any entity
    private void OnDataSourceChanged(Type changedEntityType) {
        //update Date & Time
        LastUpdate[changedEntityType] = DateTime.Now;
    }
}
public class FluffyManager
{
    private DateTime LastRead = DateTime.MinValue;

    private List<Fluffy> list;



    public List<Fluffy> GetFluffies() {

        //if first read or not uptodated
        if (list==null || !ModelWatcher.IsUpToDate(typeof(Fluffy),LastRead)) {
            list = ReadFluffies();
        }
        return list;
    }
    private List<Fluffy> ReadFluffies() { 
    //read code
    }
}