Como evitar violar o SRP em uma classe para gerenciar o cache?

12

Nota: O exemplo de código é escrito em c #, mas isso não deve importar. Coloquei c # como uma tag porque não consigo encontrar uma mais apropriada. Isso é sobre a estrutura do código.

Estou lendo o Código Limpo e tentando me tornar um programador melhor.

Muitas vezes me vejo lutando para seguir o Princípio da Responsabilidade Única (classes e funções devem fazer apenas uma coisa), especialmente em funções. Talvez o meu problema seja que "uma coisa" não esteja bem definida, mas ainda assim ...

Um exemplo: eu tenho uma lista de Fluffies em um banco de dados. Não nos importamos com o que é fofo. Eu quero uma aula para recuperar fluffies. No entanto, os fluffies podem mudar de acordo com alguma lógica. Dependendo de alguma lógica, essa classe retornará os dados do cache ou obterá as informações mais recentes do banco de dados. Poderíamos dizer que gerencia fluffies, e isso é uma coisa. Para simplificar, digamos que os dados carregados sejam válidos por uma hora e depois sejam recarregados.

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()parece-me bem. O usuário pede alguns fluffies, nós os fornecemos. Indo recuperá-los do banco de dados, se necessário, mas isso pode ser considerado parte da obtenção dos fluffies (é claro, isso é um pouco subjetivo).

NeedsReload()parece certo também. Verifica se precisamos recarregar os fluffies. UpdateNextLoad está bom. Atualiza o horário da próxima recarga. essa é definitivamente uma coisa.

No entanto, sinto que o que LoadFluffies()fazer não pode ser descrito como uma única coisa. Ele está obtendo os dados do banco de dados e agendando a próxima recarga. É difícil argumentar que calcular o tempo para a próxima recarga faz parte da obtenção dos dados. No entanto, não consigo encontrar uma maneira melhor de fazê-lo (renomear a função LoadFluffiesAndScheduleNextLoadpode ser melhor, mas isso apenas torna o problema mais óbvio).

Existe uma solução elegante para realmente escrever esta classe de acordo com o SRP? Estou sendo muito pedante?

Ou talvez minha turma não esteja realmente fazendo apenas uma coisa?

Raven
fonte
3
Baseado em "escrito em C #, mas isso não deve importar", "trata-se de estrutura de código", "um exemplo: ... não nos importamos com o que é um Fluffy", "para simplificar, digamos ...", isso não é uma solicitação de revisão de código, mas uma pergunta sobre um princípio geral de programação.
200_success 21/11/2015
@ 200_success obrigado, e desculpe, eu pensei que este seria adequado para CR
corvo
1
Tão fofinho!
Mathieu Guindon
2
No futuro, você se sairia melhor com "widget" em vez de fofo para futuras perguntas semelhantes, pois um widget é considerado um exemplo não específico para exemplos.
Whatsisname
1
Sei que é apenas um exemplo de código, mas use-o DateTime.UtcNowpara evitar trocas de horário de verão ou mesmo uma alteração no fuso horário atual.
Mark Hurd

Respostas:

23

Se essa classe realmente fosse tão trivial quanto parece, não haveria necessidade de se preocupar em violar o SRP. Então, e se uma função de 3 linhas tiver 2 linhas fazendo uma coisa e outra 1 linha fazendo outra coisa? Sim, essa função trivial viola o SRP, e daí? Quem se importa? A violação do SRP começa a se tornar um problema quando as coisas ficam mais complicadas.

Seu problema nesse caso em particular provavelmente decorre do fato de a classe ser mais complicada do que as poucas linhas que você nos mostrou.

Especificamente, o problema provavelmente reside no fato de que essa classe não apenas gerencia o cache, mas também provavelmente contém a implementação do GetFluffiesFromDb()método. Portanto, a violação do SRP está na classe, não nos poucos métodos triviais mostrados no código que você postou.

Então, aqui está uma sugestão sobre como lidar com todos os tipos de casos que se enquadram nessa categoria geral, com a ajuda do Padrão 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;
    }
}

e é usado da seguinte maneira:

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

Observe como CachingFluffiesProvider.GetFluffies()não tem medo de conter o código que verifica e atualiza o tempo, porque isso é trivial. O que esse mecanismo faz é abordar e manipular o SRP no nível de design do sistema, onde é importante, não no nível de pequenos métodos individuais, onde isso não importa.

Mike Nakis
fonte
1
+1 por reconhecer que fluffies, cache e acesso à base de dados são na verdade três responsabilidades. Você pode até tentar tornar a interface FluffiesProvider e os decoradores genéricos (IProvider <Fluffy>, ...), mas isso pode ser YAGNI.
Roman Reiner
Honestamente, se houver apenas um tipo de cache e ele sempre puxar objetos do banco de dados, o IMHO será fortemente superprojetado (mesmo que a classe "real" possa ser mais complexa, como podemos ver no exemplo). Abstração apenas por abstração não torna o código mais limpo ou mais sustentável.
Doc Brown
O problema do @DocBrown é a falta de contexto para a pergunta. Eu gosto desta resposta porque mostra uma maneira que eu usei várias vezes em aplicativos maiores e, como é fácil escrever testes, também gosto da minha resposta, porque é apenas uma pequena alteração e produz algo claro sem qualquer super design. atualmente está, sem contexto, praticamente todas as respostas aqui são boas:]
stijn
1
FWIW, a classe que eu tinha em mente quando fiz a pergunta é mais complicada do que o FluffiesManager, mas não excessivamente. Cerca de 200 linhas, talvez. Eu não fiz essa pergunta porque encontrei algum problema com meu design (ainda?), Apenas porque não consegui encontrar uma maneira de cumprir rigorosamente o SRP e isso poderia ser um problema em casos mais complexos. Portanto, a falta de contexto é um pouco pretendida. Eu acho que essa resposta é ótima.
Raven #
2
@ stijn: bem, acho que sua resposta é muito pouco atendida. Em vez de adicionar abstração desnecessária, basta cortar e nomear as responsabilidades de maneira diferente, o que deve ser sempre a primeira escolha antes de juntar três camadas de herança a um problema tão simples.
Doc Brown
6

Sua aula em si parece boa para mim, mas você está certo LoadFluffies(), não exatamente o que o nome anuncia. Uma solução simples seria alterar o nome e mover a recarga explícita de GetFluffies para uma função com uma descrição apropriada. Algo como

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

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

parece limpo para mim (também porque, como Patrick diz: é composto de outras pequenas funções obedientes ao SRP), e especialmente claro, o que às vezes é igualmente importante.

stijn
fonte
1
Eu gosto da simplicidade disso.
Raven #
6

Eu acredito que sua classe está fazendo uma coisa; é um cache de dados com um tempo limite. O LoadFluffies parece uma abstração inútil, a menos que você a chame de vários lugares. Eu acho que seria melhor pegar as duas linhas do LoadFluffies e colocá-las no condicional NeedsReload no GetFluffies. Isso tornaria a implementação do GetFluffies muito mais óbvia e ainda é um código limpo, pois você está compondo sub-rotinas de responsabilidade única para atingir um único objetivo, uma recuperação em cache de dados do banco de dados. Abaixo está o método get fluffies atualizado.

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

    return m_Cache;
}
Patrick Goley
fonte
Embora essa seja uma boa primeira resposta, lembre-se de que o código "resultado" geralmente é uma boa adição.
Fund Monica's Lawsuit
4

Seus instintos estão corretos. Sua turma, por menor que seja, está fazendo muito. Você deve separar a lógica de cache de atualização programada em uma classe completamente genérica. Em seguida, crie uma instância específica dessa classe para gerenciar Fluffies, algo como isto (não compilado, o código de trabalho é deixado como um exercício para o leitor):

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

Uma vantagem adicional é que agora é muito fácil testar o TimedRefreshCache.

Kevin Cline
fonte
1
Concordo que, se a lógica de atualização ficar mais complicada do que no exemplo, pode ser uma boa ideia refatorá-la em uma classe separada. Mas eu discordo que a classe no exemplo, como é, faz muito.
Doc Brown
@ Kevin, eu não tenho experiência em TDD. Você poderia elaborar como testaria o TimedRefreshCache? Não vejo isso como "muito fácil", mas pode ser minha falta de conhecimento.
Raven #
1
Pessoalmente, não gosto da sua resposta por causa de sua complexidade. É muito genérico e muito abstrato e pode ser melhor em situações mais complicadas. Mas neste caso simples, é "simplesmente demais". Por favor, dê uma olhada na resposta de stijn. Que resposta agradável, curta e legível. Todo mundo vai entender isso imediatamente. O que você acha?
Dieter Meemken
1
@raven Você pode testar o TimedRefreshCache usando um intervalo curto (como 100 ms) e um produtor muito simples (como DateTime.Now). A cada 100 ms, o cache produzirá um novo valor, retornando o valor anterior.
Kevin cline
1
@ DocBrown: O problema é que, como está escrito, não pode ser testado. A lógica de cronometragem (testável) é acoplada à lógica do banco de dados, que muito então é ridicularizada. Depois que uma costura é criada para simular a chamada do banco de dados, você está 95% do caminho para a solução genérica. Eu descobri que a construção dessas turmas geralmente compensa, porque acaba sendo reutilizada mais do que o esperado.
Kevin cline
1

Sua classe é boa, o SRP é sobre uma classe e não uma função, toda a classe é responsável por fornecer os "Fluffies" da "Fonte de Dados" para que você seja livre na implementação interna.

Se você deseja expandir o mecanismo de conversão, pode criar uma classe respeitável para assistir à fonte de dados

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
    }
}
yousif.aljamri
fonte
De acordo com o tio Bob: as funções devem fazer uma coisa. Eles deveriam fazê-lo bem. Eles devem fazê-lo apenas. Código Limpo p.35.
Raven #