¿Cómo evitar violar el SRP en una clase para administrar el almacenamiento en caché?

Nota: El código de muestra está escrito en C #, pero eso no debería importar. Puse c # como etiqueta porque no puedo encontrar una más adecuada. Esto se trata de la estructura del código.

Estoy leyendo Clean Code e intento convertirme en un mejor programador.

A menudo me encuentro luchando por seguir el Principio de Responsabilidad Única (las clases y las funciones deberían hacer solo una cosa), especialmente en las funciones. Quizás mi problema es que "una cosa" no está bien definida, pero aún así ...

Un ejemplo: tengo una lista de Fluffies en una base de datos. No nos importa lo que sea un Fluffy. Quiero una clase para recuperar pelusas. Sin embargo, las pelusas pueden cambiar según alguna lógica. Dependiendo de alguna lógica, esta clase devolverá los datos del caché u obtendrá lo último de la base de datos. Podríamos decir que maneja pelusas, y eso es una cosa. Para simplificarlo, supongamos que los datos cargados son válidos durante una hora y luego deben volver a cargarse.

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()me parece bien El usuario solicita algunas pelusas, nosotros se las proporcionamos. Ir a recuperarlos de la base de datos si es necesario, pero eso podría considerarse parte de obtener las esponjosas (por supuesto, eso es algo subjetivo).

NeedsReload()Parece correcto también. Comprueba si necesitamos recargar las pelusas. UpdateNextLoad está bien. Actualiza el tiempo para la próxima recarga. Eso es definitivamente una sola cosa.

Sin embargo, siento que LoadFluffies()no se puede describir como una sola cosa. Está obteniendo los datos de la base de datos, y está programando la próxima recarga. Es difícil argumentar que calcular el tiempo para la próxima recarga es parte de obtener los datos. Sin embargo, no puedo encontrar una mejor manera de hacerlo (cambiar el nombre de la función LoadFluffiesAndScheduleNextLoadpuede ser mejor, pero solo hace que el problema sea más obvio).

¿Existe una solución elegante para escribir realmente esta clase de acuerdo con el SRP? ¿Estoy siendo demasiado pedante?

¿O tal vez mi clase no está haciendo solo una cosa?

Si esta clase fuera realmente tan trivial como parece ser, entonces no habría necesidad de preocuparse por violar el SRP. Entonces, ¿qué pasa si una función de 3 líneas tiene 2 líneas haciendo una cosa y otra 1 línea haciendo otra cosa? Sí, esta función trivial viola el SRP, ¿y qué? ¿A quien le importa? La violación del SRP comienza a convertirse en un problema cuando las cosas se vuelven más complicadas.

Su problema en este caso particular probablemente se deba al hecho de que la clase es más complicada que las pocas líneas que nos ha mostrado.

Específicamente, el problema probablemente radica en el hecho de que esta clase no solo administra el caché, sino que también contiene la implementación del GetFluffiesFromDb()método. Entonces, la violación del SRP está en la clase, no en los pocos métodos triviales que se muestran en el código que publicó.

Entonces, aquí hay una sugerencia sobre cómo manejar todo tipo de casos que caen dentro de esta categoría general, con la ayuda del Patrón Decorador .

/// 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;
    }
}

y se usa de la siguiente manera:

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

Tenga en cuenta que CachingFluffiesProvider.GetFluffies()no tiene miedo de contener el código que verifica y actualiza el tiempo, porque eso es algo trivial. Lo que hace este mecanismo es abordar y manejar el SRP a nivel de diseño del sistema, donde importa, no a nivel de pequeños métodos individuales, donde de todos modos no importa.

Tu clase en sí me parece bien, pero tienes razón en eso LoadFluffies()no es exactamente lo que anuncia el nombre. Una solución simple sería cambiar el nombre y mover la recarga explícita de GetFluffies a una función con una descripción adecuada. Algo como

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

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

A mí me parece limpio (también porque, como dice Patrick: está compuesto de otras pequeñas funciones obedientes a SRP), y especialmente claro, que a veces es tan importante.

Creo que tu clase está haciendo una cosa; Es un caché de datos con un tiempo de espera. LoadFluffies parece una abstracción inútil a menos que lo llame desde varios lugares. Creo que sería mejor tomar las dos líneas de LoadFluffies y ponerlas en el condicional NeedsReload en GetFluffies. Esto haría que la implementación de GetFluffies sea mucho más obvia y siga siendo un código limpio, ya que está componiendo subrutinas de responsabilidad única para lograr un solo objetivo, una recuperación en caché de datos de la base de datos. A continuación se muestra el método actualizado get fluffies.

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

    return m_Cache;
}

Tus instintos son correctos. Tu clase, por pequeña que sea, está haciendo demasiado. Debe separar la lógica de almacenamiento en caché de actualización temporizada en una clase completamente genérica. Luego cree una instancia específica de esa clase para administrar Fluffies, algo como esto (no compilado, el código de trabajo se deja como ejercicio para el lector):

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; } }
}

Una ventaja adicional es que ahora es muy fácil probar TimedRefreshCache.

Su clase está bien, SRP se trata de una clase, no de una función, toda la clase es responsable de proporcionar los "Fluffies" de la "Fuente de datos" para que tenga una implementación interna libre.

Si desea expandir el mecanismo de cahing, puede crear una clase respirable para ver la fuente de datos

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
    }
}