struct com valor padrão sem sentido

12

No meu sistema eu freqüentemente operar com códigos de aeroporto ( "YYZ", "LAX", "SFO", etc.), eles estão sempre no mesmo formato exato (3 letras, representado como maiúsculas). O sistema normalmente lida com 25 a 50 desses códigos (diferentes) por solicitação da API, com mais de mil alocações no total, eles são passados ​​por várias camadas de nosso aplicativo e são comparados quanto à igualdade com bastante frequência.

Começamos apenas passando as strings, o que funcionou bem por um tempo, mas rapidamente notamos muitos erros de programação, passando um código errado em algum lugar em que o código de 3 dígitos era esperado. Também enfrentamos problemas em que deveríamos fazer uma comparação sem distinção entre maiúsculas e minúsculas e, em vez disso, não resultando em erros.

A partir disso, decidi parar de passar as strings e criar uma Airportclasse, que tem um único construtor que pega e valida o código do aeroporto.

public sealed class Airport
{
    public Airport(string code)
    {
        if (code == null)
        {
            throw new ArgumentNullException(nameof(code));
        }

        if (code.Length != 3 || !char.IsLetter(code[0]) 
        || !char.IsLetter(code[1]) || !char.IsLetter(code[2]))
        {
            throw new ArgumentException(
                "Must be a 3 letter airport code.", 
                nameof(code));
        }

        Code = code.ToUpperInvariant();
    }

    public string Code { get; }

    public override string ToString()
    {
        return Code;
    }

    private bool Equals(Airport other)
    {
        return string.Equals(Code, other.Code);
    }

    public override bool Equals(object obj)
    {
        return obj is Airport airport && Equals(airport);
    }

    public override int GetHashCode()
    {
        return Code?.GetHashCode() ?? 0;
    }

    public static bool operator ==(Airport left, Airport right)
    {
        return Equals(left, right);
    }

    public static bool operator !=(Airport left, Airport right)
    {
        return !Equals(left, right);
    }
}

Isso tornou nosso código muito mais fácil de entender e simplificamos nossas verificações de igualdade, uso de dicionário / conjunto. Agora sabemos que, se nossos métodos aceitarem uma Airportinstância que se comportará da maneira que esperamos, simplificou nossas verificações de método para uma verificação de referência nula.

O que notei, no entanto, foi que a coleta de lixo estava sendo executada com muito mais frequência, que eu rastreei para muitas instâncias de Airportcoleta.

Minha solução para isso foi converter o arquivo classem struct. Principalmente foi apenas uma alteração de palavra-chave, com exceção de GetHashCodee ToString:

public override string ToString()
{
    return Code ?? string.Empty;
}

public override int GetHashCode()
{
    return Code?.GetHashCode() ?? 0;
}

Para lidar com o caso em que default(Airport)é usado.

Minhas perguntas:

  1. A criação de uma Airportclasse ou estrutura foi uma boa solução em geral, ou estou resolvendo o problema errado / resolvendo-o da maneira errada, criando o tipo? Se não é uma boa solução, qual é a melhor solução?

  2. Como meu aplicativo deve lidar com instâncias em que o default(Airport)arquivo é usado? Um tipo de default(Airport)é absurdo para o meu aplicativo, então eu tenho feito if (airport == default(Airport) { throw ... }em lugares onde obter uma instância de Airport(e sua Codepropriedade) é fundamental para a operação.

Notas: Revisei as perguntas C # / VB struct - como evitar casos com zero valor padrão, que é considerado inválido para determinada estrutura? , e Use struct ou não antes de fazer minha pergunta, no entanto, acho que minhas perguntas são diferentes o suficiente para garantir sua própria postagem.

Mateus
fonte
7
A coleta de lixo tem um impacto material no desempenho do seu aplicativo? Em outras palavras, isso importa?
Robert Harvey
Enfim, sim, a solução de classe foi "boa". A maneira como você sabe disso é que resolveu o seu problema sem criar novos.
Robert Harvey
2
Uma maneira de resolver o default(Airport)problema é simplesmente desautorizar as instâncias padrão. Você pode fazer isso escrevendo um construtor sem parâmetros e jogando InvalidOperationExceptionou NotImplementedExceptionnele.
Robert Harvey
3
Em uma nota lateral, em vez de confirmar que sua sequência de inicialização é de fato três caracteres alfa, por que não compará-la com a lista finita de todos os códigos de aeroportos (por exemplo, github.com/datasets/airport-codes ou similar)?
Dan Pichelman
2
Estou disposto a apostar várias cervejas que essa não é a raiz de um problema de desempenho. Um laptop normal pode alocar na ordem de 10 milhões de objetos / segundo.
Esben Skov Pedersen

Respostas:

6

Atualização: reescrevi minha resposta para abordar algumas suposições incorretas sobre estruturas C #, bem como o OP nos informando nos comentários que as cadeias internas estão sendo usadas.


Se você pode controlar os dados que chegam ao seu sistema, use uma classe conforme publicado na sua pergunta. Se alguém correr default(Airport), receberá um nullvalor de volta. Certifique-se de escrever seu Equalsmétodo privado para retornar false sempre que comparar objetos nulos do Airport e, em seguida, deixe os NullReferenceExceptionvoarem para outro lugar no código.

No entanto, se você estiver levando dados para o sistema a partir de fontes que não controla, não será necessário travar todo o encadeamento. Nesse caso, uma estrutura é ideal, pois o fato simples default(Airport)fornecerá algo diferente de um nullponteiro. Crie um valor óbvio para representar "sem valor" ou o "valor padrão" para ter algo para imprimir na tela ou em um arquivo de log (como "---", por exemplo). Na verdade, eu apenas manteria o codeprivado e não exporia uma Codepropriedade - apenas me focaria no comportamento aqui.

public struct Airport
{
    private string code;

    public Airport(string code)
    {
        // Check `code` for validity, throw exceptions if not valid

        this.code = code;
    }

    public override string ToString()
    {
        return code ?? (code = "---");
    }

    // int GetHashcode()

    // bool Equals(...)

    // bool operator ==(...)

    // bool operator !=(...)

    private bool Equals(Airport other)
    {
        if (other == null)
            // Even if this method is private, guard against null pointers
            return false;

        if (ToString() == "---" || other.ToString() == "---")
            // "Default" values should never match anything, even themselves
            return false;

        // Do a case insensitive comparison to enforce logic that airport
        // codes are not case sensitive
        return string.Equals(
            ToString(),
            other.ToString(),
            StringComparison.InvariantCultureIgnoreCase);
    }
}

No pior cenário possível, a conversão default(Airport)em uma seqüência de caracteres é impressa "---"e retorna false quando comparado a outros códigos de aeroporto válidos. Qualquer código de aeroporto "padrão" não corresponde a nada, incluindo outros códigos de aeroporto padrão.

Sim, as estruturas devem ser valores alocados na pilha, e qualquer ponteiro para acumular memória basicamente nega as vantagens de desempenho das estruturas, mas, nesse caso, o valor padrão de uma estrutura tem significado e fornece alguma resistência a marcadores adicionais para o restante do inscrição.

Eu dobraria as regras um pouco aqui, por causa disso.


Resposta original (com alguns erros factuais)

Se você pode controlar os dados que chegam ao seu sistema, eu faria o que Robert Harvey sugeriu nos comentários: Crie um construtor sem parâmetros e lance uma exceção quando for chamada. Isso impede que dados inválidos entrem no sistema via default(Airport).

public Airport()
{
    throw new InvalidOperationException("...");
}

No entanto, se você estiver levando dados para o sistema a partir de fontes que você não controla, não será necessário travar todo o encadeamento. Nesse caso, você pode criar um código de aeroporto inválido, mas parecer um erro óbvio. Isso envolveria a criação de um construtor sem parâmetros e a configuração Codepara algo como "---":

public Airport()
{
    Code = "---";
}

Como você está usando a stringcomo o código, não faz sentido usar uma estrutura. A estrutura é alocada na pilha, apenas para Codealocá-la como ponteiro para uma seqüência de caracteres na memória heap, portanto, não há diferença aqui entre classe e estrutura.

Se você alterasse o código do aeroporto para uma matriz de 3 itens de caracteres, uma estrutura seria totalmente alocada na pilha. Mesmo assim, o volume de dados não é tão grande para fazer a diferença.

Greg Burghardt
fonte
Se meu aplicativo estivesse usando seqüências de caracteres internas para a Codepropriedade, isso mudaria sua justificativa em relação ao fato de o ponto da sequência estar na memória de pilha?
Matthew
@ Matthew: Está usando uma classe dando a você um problema de desempenho? Caso contrário, jogue uma moeda para decidir qual usar.
Greg Burghardt
4
@ Matthew: Realmente o importante é que você centralizou a lógica problemática de normalizar os códigos e comparações. Depois disso, "classe versus estrutura" é apenas uma discussão acadêmica, até você medir um impacto suficientemente grande no desempenho para justificar o tempo extra do desenvolvedor para ter uma discussão acadêmica.
Greg Burghardt 10/09
1
É verdade, não me importo de ter uma discussão acadêmica de tempos em tempos se isso me ajudar a criar soluções mais bem informadas no futuro.
Matthew
@ Matthew: Sim, você está absolutamente certo. Eles dizem que "falar é barato". Certamente é mais barato do que não falar e construir algo ruim. :)
Greg Burghardt
13

Use o padrão Flyweight

Como o Airport é, corretamente, imutável, não há necessidade de criar mais de uma instância de uma determinada, por exemplo, SFO. Use um Hashtable ou similar (observe, eu sou um cara Java, não C #, para que detalhes exatos possam variar), para armazenar em cache os aeroportos quando eles forem criados. Antes de criar um novo, verifique no Hashtable. Você nunca está liberando aeroportos, portanto o GC não precisa liberá-los.

Uma pequena vantagem adicional (pelo menos em Java, não tenho certeza sobre C #) é que você não precisa escrever um equals()método, ==basta fazer isso. O mesmo para hashcode().

user949300
fonte
3
Excelente uso do padrão flyweight.
Neil
2
Supondo que o OP continue usando uma estrutura e não uma classe, a cadeia de caracteres de cadeia de caracteres já não está manipulando os valores de cadeia de caracteres reutilizáveis? As estruturas já estão na pilha, as cadeias já estão sendo reutilizadas para evitar valores duplicados na memória. Que benefício adicional seria obtido com o padrão de peso de mosca?
Flater
Algo a observar. Se um aeroporto for adicionado ou removido, você precisará criar uma maneira de atualizar essa lista estática sem reiniciar o aplicativo ou reimplementá-lo. Os aeroportos não são adicionados ou removidos com frequência, mas os empresários tendem a ficar um pouco chateados quando uma simples mudança se torna complicada. "Não posso simplesmente adicioná-lo a algum lugar ?! Por que temos que agendar uma reinicialização de versão / aplicativo e incomodar nossos clientes?" Mas eu também estava pensando em usar algum tipo de cache estático no começo.
Greg Burghardt
@Flater Ponto razoável. Eu diria que menos necessidade de programadores juniores raciocinar sobre pilha versus pilha. Além disso, veja minha adição - não há necessidade de escrever igual a ().
user949300
1
@ Greg Burghardt Se o getAirportOrCreate()código estiver sincronizado corretamente, não há motivo técnico para você não criar novos aeroportos conforme necessário durante o tempo de execução. Pode haver razões comerciais.
user949300
3

Eu não sou um programador particularmente avançado, mas isso não seria um uso perfeito para um Enum?

Existem diferentes maneiras de construir classes enum a partir de listas ou seqüências de caracteres. Aqui está um que eu vi no passado, mas não tenho certeza se é o melhor caminho.

https://blog.kloud.com.au/2016/06/17/converting-webconfig-values-into-enum-or-list/

Adam B
fonte
2
Quando há potencialmente milhares de valores diferentes (como é o caso dos códigos de aeroportos), um enum simplesmente não é prático.
Ben Cottrell
Sim, mas o link que eu postei é como carregar seqüências de caracteres como enumerações. Aqui está outro link para carregar uma tabela de pesquisa como uma enumeração. Pode ser um pouco trabalhoso, mas tiraria vantagem do poder das enumerações. exceptionnotfound.net/...
Adam B
1
Ou uma lista de códigos válidos pode ser carregada de um banco de dados ou arquivo. Em seguida, apenas um código de aeroporto é verificado para estar entre essa lista. É o que você normalmente faz quando não deseja mais codificar os valores e / ou a lista fica muito longa para gerenciar.
Neil
@BenCottrell, que é exatamente para que servem os modelos de geração de código e T4.
11338 RubberDuck
3

Um dos motivos pelos quais você está vendo mais atividades no GC é porque está criando uma segunda sequência agora - a .ToUpperInvariant()versão da sequência original. A sequência original é elegível para o GC logo após a execução do construtor e a segunda é elegível ao mesmo tempo que o Airportobjeto. Você pode minimizá-lo de uma maneira diferente (observe o terceiro parâmetro em string.Equals()):

public sealed class Airport : IEquatable<Airport>
{
    public Airport(string code)
    {
        if (code == null)
        {
            throw new ArgumentNullException(nameof(code));
        }

        if (code.Length != 3 || !char.IsLetter(code[0])
                             || !char.IsLetter(code[1]) || !char.IsLetter(code[2]))
        {
            throw new ArgumentException(
                "Must be a 3 letter airport code.",
                nameof(code));
        }

        Code = code;
    }

    public string Code { get; }

    public override string ToString()
    {
        return Code; // TODO: Upper-case it here if you really need to for display.
    }

    public bool Equals(Airport other)
    {
        return string.Equals(Code, other?.Code, StringComparison.InvariantCultureIgnoreCase);
    }

    public override bool Equals(object obj)
    {
        return obj is Airport airport && Equals(airport);
    }

    public override int GetHashCode()
    {
        return Code.GetHashCode();
    }

    public static bool operator ==(Airport left, Airport right)
    {
        return Equals(left, right);
    }

    public static bool operator !=(Airport left, Airport right)
    {
        return !Equals(left, right);
    }
}
Jesse C. Slicer
fonte
Isso não gera códigos de hash diferentes para aeroportos iguais (mas com letras maiúsculas diferentes)?
Hero Wanders
Sim, eu imagino que sim. Que droga.
Jesse C. Slicer
Este é um ponto muito bom, nunca pensei nisso, vou olhar para fazer essas mudanças.
Matthew
1
No que diz respeito a GetHashCode, deve apenas usar StringComparer.OrdinalIgnoreCase.GetHashCode(Code)ou similar
Matthew