Pergunta

Eu tenho um objeto de valor imutável, IPathwayModule, cujo valor é definido por:

  • (int) Bloco;
  • (Entidade) Módulo, identificado por (string) ModuleId;
  • (enum) Status;e
  • (entidade) Classe, identificada por (string) ClassId - que pode ser nula.

Aqui está minha implementação atual do IEqualityComparer, que parece funcionar em alguns testes de unidade.No entanto, acho que não entendo o que estou fazendo bem o suficiente para saber se estou fazendo certo.Uma implementação anterior às vezes falhava em testes repetidos.

private class StandardPathwayModuleComparer : IEqualityComparer<IPathwayModule>
{
    public bool Equals(IPathwayModule x, IPathwayModule y)
    {
        int hx = GetHashCode(x);
        int hy = GetHashCode(y);
        return hx == hy;
    }

    public int GetHashCode(IPathwayModule obj)
    {
        int h;
        if (obj.Class != null)
        {
            h = obj.Block.GetHashCode() + obj.Module.ModuleId.GetHashCode() + obj.Status.GetHashCode() + obj.Class.ClassId.GetHashCode();
        }
        else
        {
            h = obj.Block.GetHashCode() + obj.Module.ModuleId.GetHashCode() + obj.Status.GetHashCode() + "NOCLASS".GetHashCode();
        }
        return h;
    }
}

IPathwayModule é definitivamente imutável e instâncias diferentes com os mesmos valores devem ser iguais e produzir o mesmo HashCode, uma vez que são usadas como itens em HashSets.

Suponho que minhas perguntas sejam:

  • Estou usando a interface corretamente neste caso?
  • Existem casos em que posso não ver o comportamento desejado?
  • Existe alguma maneira de melhorar a robustez e o desempenho?
  • Há alguma boa prática que não estou seguindo?
Foi útil?

Solução 7

Obrigado a todos que responderam.Agregei o feedback de todos que responderam e minha melhoria IEqualityComparer agora parece:

private class StandardPathwayModuleComparer : IEqualityComparer<IPathwayModule>
{
    public bool Equals(IPathwayModule x, IPathwayModule y)
    {
        if (x == y) return true;
        if (x == null || y == null) return false;

        if ((x.Class == null) ^ (y.Class == null)) return false;

        if (x.Class == null) //and implicitly y.Class == null
        {
            return x.Block.Equals(y.Block) && x.Status.Equals(y.Status) && x.Module.ModuleId.Equals(y.Module.ModuleId);
        }
        return x.Block.Equals(y.Block) && x.Status.Equals(y.Status) && x.Module.ModuleId.Equals(y.Module.ModuleId) && x.Class.ClassId.Equals(y.Class.ClassId);
    }
    public int GetHashCode(IPathwayModule obj)
    {
        unchecked {
            int h = obj.Block ^ obj.Module.ModuleId.GetHashCode() ^ (int) obj.Status;
            if (obj.Class != null)
            {
               h ^= obj.Class.ClassId.GetHashCode();
            }
            return h;
        }
    }
}

Outras dicas

Não faça o Equals em termos dos resultados da função Hash, pois é muito frágil.Em vez disso, faça uma comparação de valores de campo para cada um dos campos.Algo como:

return x != null && y != null && x.Name.Equals(y.Name) && x.Type.Equals(y.Type) ...

Além disso, os resultados das funções hash não são realmente passíveis de adição.Tente usar o ^ operador em vez disso.

return obj.Name.GetHashCode() ^ obj.Type.GetHashCode() ...

Você não precisa da verificação nula em GetHashCode.Se esse valor for nulo, você tem problemas maiores, não adianta tentar se recuperar de algo sobre o qual você não tem controle...

O único grande problema é a implementação do Equals.Os códigos hash não são exclusivos; você pode obter o mesmo código hash para objetos diferentes.Você deve comparar cada campo do IPathwayModule individualmente.

GetHashCode() pode ser um pouco melhorado.Você não precisa chamar GetHashCode() em um int.O int em si é um bom código hash.O mesmo para valores enum.Seu GetHashCode poderia então ser implementado assim:

public int GetHashCode(IPathwayModule obj)
{
    unchecked {
        int h = obj.Block + obj.Module.ModeleId.GetHashCode() + (int) obj.Status;
        if (obj.class != null)
           h += obj.Class.ClassId.GetHashCode();
        return h;
    }
}

O bloco 'unchecked' é necessário porque pode haver overflows nas operações aritméticas.

Você não deve usar GetHashCode() como principal forma de comparação de objetos.Compare-o em termos de campo.

Pode haver vários objetos com o mesmo código hash (isso é chamado de 'colisões de código hash').

Além disso, tenha cuidado ao somar vários valores inteiros, pois você pode facilmente causar uma OverflowException.Use 'exclusivo ou' (^) para combinar códigos hash ou agrupar o código em um bloco 'desmarcado'.

Você deve implementar versões melhores de Equals e GetHashCode.

Por exemplo, o código hash de enums é simplesmente seu valor numérico.

Em outras palavras, com essas duas enumerações:

public enum A { x, y, z }
public enum B { k, l, m }

Então, com sua implementação, o seguinte tipo de valor:

public struct AB {
    public A;
    public B;
}

os dois valores a seguir seriam considerados iguais:

AB ab1 = new AB { A = A.x, B = B.m };
AB ab2 = new AB { A = A.z, B = B.k };

Presumo que você não queira isso.

Além disso, passar os tipos de valor como interfaces irá encaixá-los, o que pode causar problemas de desempenho, embora provavelmente não muitos.Você pode considerar fazer com que a implementação do IEqualityComparer receba seus tipos de valor diretamente.

  1. Assumir que dois objetos são iguais porque seu código hash é igual é errado.Você precisa comparar todos os membros individualmente
  2. Provavelmente é melhor usar ^ em vez de + para combinar os códigos hash.

Se bem entendi, você gostaria de ouvir alguns comentários sobre seu código.Aqui estão minhas observações:

  1. GetHashCode devem ser XOR juntos, não adicionados.XOR (^) dá uma melhor chance de evitar colisões
  2. Você compara códigos hash.Isso é bom, mas faça isso apenas se o objeto subjacente substituir o GetHashCode.Caso contrário, use propriedades e seus códigos hash e combine-os.
  3. Os códigos hash são importantes, pois possibilitam uma comparação rápida.Mas se os códigos hash forem iguais, o objeto ainda poderá ser diferente.Isso acontece raramente.Mas você precisará comparar os campos do seu objeto se os códigos hash forem iguais.
  4. Você diz que seus tipos de valor são imutáveis, mas faz referência a objetos (.Class), que não são imutáveis
  5. Sempre otimize a comparação adicionando comparação de referência como primeiro teste.Referências desiguais, os objetos são desiguais, então as estruturas são desiguais.

O ponto 5 depende se você deseja que os objetos aos quais você faz referência em seu tipo de valor retornem diferentes quando não forem a mesma referência.

EDITAR: você compara muitas strings.A comparação de strings é otimizada em C#.Você pode, como outros sugeriram, usar melhor == com eles em sua comparação.Para o GetHashCode, use OU ^ como sugerido por outros também.

Licenciado em: CC-BY-SA com atribuição
Não afiliado a StackOverflow
scroll top