12
votes

Meilleure façon de savoir si iEnumerable <> a des valeurs uniques

J'ai beaucoup de code dans lequel je fais quelque chose comme ça xxx

existe-t-il une meilleure façon plus belle de faire cela?


1 commentaires

TYPO: Vous avez capitalisé des "valeurs" dans votre corps de méthode, mais votre paramètre est minuscule.


7 Réponses :


19
votes

Votre méthode a besoin de itérer à deux reprises la séquence, avec quelques inconvénients potentiels:

  1. itération à deux reprises sera plus lent que itérant une fois pour les séquences de toute taille significative.
  2. Certaines séquences vont lancer une exception si vous essayez de les introduire plus d'une fois; D'autres peuvent renvoyer différents résultats pour des itérations ultérieures.
  3. Votre méthode utilise Nombre qui doit itérer toute la séquence à chaque fois. Il n'y a aucune raison pour que vous ne devriez pas vous sortir tôt dès que vous savez qu'il y a une valeur en double.

    La méthode suivante doit uniquement se déplacer à travers la séquence une fois, et va sortir tôt dès que toute valeur en double est rencontrée: xxx


0 commentaires

22
votes

Je ferais cela une belle méthode d'extension xxx

vérifie que tous les éléments peuvent être ajoutés à un hashset.


11 commentaires

+1 Utilisation de l'HS.Ajouter comme le paramètre est très beau (si seulement un peu illisible). (et mettre un espace entre var et hs)


Belle réponse, même si un peu te terne :)


Je n'aime pas avoir un prédicat à tout avec des effets secondaires.


Oui, je suis d'accord avec @codeinchaos, les méthodes LINQ ne sont pas censées provoquer des effets secondaires ...


L'effet secondaire est assez non pertinent. Le hashset est jeté après avoir sorti la méthode.


Je n'ai pas dit que c'était incorrect, seulement que c'est mauvais style. Par exemple, imaginez une implémentation de tout qui signe le résultat du prédicat. Cela rompt votre code.


Mais .... la mise en œuvre de tout ne signifie pas mémoriser le résultat. Envisagez-vous sérieusement une mise en œuvre malveillante de tous? Sainte vachette.


Je pense que cet exemple de code est important car il montre que vous n'avez pas besoin de faire la liste.Tous (P => H.Ajoud (P)) (cela aurait été beaucoup plus lisible, j'ajouterai) lorsque la méthode Vous voulez utiliser a la même "signature" dont vous avez besoin.


@Jamiec "mais ... la mise en œuvre de tous ne mémorise pas le résultat." Que par lui-même n'est pas pertinent. La mise en œuvre est par définition un détail de mise en œuvre. La question est la suivante: est-ce que le contrat de tout garantie travailler avec un prédicat ayant des effets secondaires. --- a accordé qu'il est peu probable que les futures versions de Linq-to-objets vont casser votre code. Mais cela ne fonctionnera peut-être pas déjà avec ordonnée de Linq parallèle. Et l'OMI Memoization est une optimisation valide dans une bibliothèque de programmation fonctionnelle telle que LINQ. Donc, je ne considérerais pas une telle mise en œuvre malveillante.


Et il y a des méthodes LINQ présentant des effets secondaires dans le prédicat peut entraîner un comportement différent entre Mono et .NET.


Je peux voir le point de codéinchaos ... pour ceux qui ne savent pas quelle est la mémoisation (et il y a une bonne wikie pour cela), disons que tous pouvaient être mis en œuvre comme return list.distinct (). (p =>! Condition) . Ce serait ok du POV du contrat, mais cela briserait cette mise en œuvre. Le problème avec ce "jeu" est qu'il existe probablement plus de classes "légèrement" définies que les classes complètement définies dans C #.



1
votes

Vous feriez deux boucles à travers les données de ce qui précède - une fois pour obtenir le compte, une fois pour obtenir le nombre distinct. Particulièrement mauvais si les deux premiers articles sont identiques! Essayez quelque chose comme ceci:

bool GetIsUnique<T>(IEnumerable<T> values)
{
    HashSet<T> hashSet = new HashSet<T>();
    foreach(var value in values)
    {
        if (hashSet.Contains(value))
        {
            return false;
        }
        hashSet.Add(value);
    }
    return true;
}


2 commentaires

Vous pouvez sûrement accélérer cela en fusionnant le hashset contient et ajoute en une étape?


Ouais, évident maintenant tu le dis! Je pensais clairement en termes d'un dictionnaire et non d'un ensemble ...



0
votes

Deux règles de base:

  1. Le moyen le plus simple de lire et de comprendre est presque toujours le meilleur moyen de coder quelque chose. Ce code est délicieusement facile à lire, alors je dirais que vous êtes prêt à partir.
  2. performance ("plus rapide") ne devrait être une préoccupation que si vous pouvez prouver que cette est la méthode qui ralentit votre programme ou si vous construisez une bibliothèque que d'autres personnes auront accès sans avoir accès au code source.

    Les autres méthodes vont être plus rapides (elles vont court-circuiter lorsqu'une valeur répétée est trouvée, renvoyant false), mais je collerais toujours votre version s'il s'agissait de mon code.


1 commentaires

La règle prééminente devrait être que "la correction est plus importante que la lisibilité ou la performance"; Si votre code est brisé, cela n'a pas d'importance à quel point il est beau ou performant. Le code dans la question est (théoriquement) cassé car toutes les séquences ne sont pas enrichies d'une seule fois.



5
votes

Je pense que cela dépend de ce que vous voulez faire s'il y a des valeurs non uniques. @ Jamiec ou la réponse de @ Lukeh sont de grandes réponses et probablement meilleures pour une vitesse pure, mais cela ne peut pas vous dire où sont les problèmes.

Vous pouvez également envisager quelque chose comme P>

var group = values.GroupBy(x => x);
return group.Where(g => g.Count() > 1).Select(g => g.Key);


7 commentaires

@Digemall: Utilisation de GroupBy nécessite que toute la séquence soit itératée avant de commencer à tester des doublons, ce qui pourrait être un problème lorsqu'il s'agit de séquences plus longues. (Le hashset Les implémentations peuvent sortir dès que la première DUPE est trouvée.)


@Lukeh - yup, hashset sera toujours plus rapide si vous souhaitez simplement répondre à la question "Existe-t-il des duplicats?". J'essaie juste de souligner que si vous allez faire un suivi avec «Quels sont les doublons?» ou "combien de doublons y a-t-il?" Ensuite, il vaut la peine d'envisager d'utiliser GroupBy car vous pouvez la poser toutes ces questions.


@Mike: Mais en utilisant groupby pour la requête initiale ne vous donne aucun avantage. La requête sera réévaluée chaque fois que vous l'utilisez, alors pourquoi ne pas utiliser intersect pour le test initial, puis groupby pour toutes les requêtes ultérieures, si vous en avez besoin?


@Lukeh: Nous faisons quelque chose de similaire sur mon projet actuel. Pour nous, l'un des gros coûts est la comparaison d'égalité et gethashcode appels. Avec le GroupBy appelé une fois que nous ne payons que ce coût pour une boucle via la collection d'origine. Ce que nous faisons, c'est appeler groupby , sélectionnez les groupes avec plusieurs éléments et appelez tolist à ce sujet. Nous travaillons ensuite beaucoup sur les groupes avec des doublons. Ne pas réclamer cela est juste dans toutes les situations. J'essaie juste de donner une alternative et des raisons de le considérer. Seules performances de la manière doivent être mesurées pour votre situation réellement.


@Mike: assez juste, mais les exemples de votre réponse ne le font pas . La variable (code> (code> est une description d'une requête, pas un ensemble de résultats, donc chaque fois que groupe est utilisé alors la requête groupby sera exécutée encore.


@Lukeh: bon point. J'ai trop simplifié dans la réponse. Je vais mettre dans une version plus complète pour clarifier. Merci pour les commentaires!


@Lukeh: Oui, bien sûr, il évalue toute la liste, donc il est plus lent (à l'exclusion du pire des cas) ... mais comme indiqué par Mike si vous devez savoir quels sont les duplicats qui le permettent (évidemment, vous devez sauvegarder le mettre en doute)...



0
votes

Un moyen rapide de trouver le premier duplicata, le cas échéant, est le suivant: xxx


0 commentaires

0
votes

Je suis surpris que personne n'a testé cela simplement:

comparer la version de GuiIP dans la question avec Jamiec, Lukek et MRKwatkins, toutes les trois réponses sont meilleures que la version de l'Asker. P>

de Les trois réponses, elles sont toutes assez comparables, mais dans la plupart des cas, Jamiec est légèrement plus rapide. p>

Lorsque les données n'ont aucun doublon ou un duplicata, la taille ou l'algorithme n'avait aucune différence majeure. P>

Lorsque les données ont une duplicate près du milieu, Ou au début, la version de Guip dans la question initiale montre sa lenteur par rapport aux trois autres. p>

Le temps de vérification d'un jeu apparaît à une échelle linéaire avec la taille de la définition, ce qui signifie qu'aucun algorithme n'est préférable pour les grands ou les petits ensembles. P>

Voici un programme de test qui croit une CSV que vous pouvez charger dans un programme de feuille de calcul et trier et graphiquement si vous aimez: p>

Programme de test: p>

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace AreUniqueTest
{
class Program
{
    const int Iterations = 1000;

    enum DupeLocation
    {
        None,
        Early,
        Center,
        Late,
    }

    enum SetSize
    {
        Tiny= 10,
        Small = 100,
        Medium = 1000,
        Large = 10000,
        Huge = 100000,
    }

    static void Main()
    {
        Dictionary<string, Func<IEnumerable<int>, bool>> functions = new Dictionary<string, Func<IEnumerable<int>, bool>>
        {
            {"Gluip", GetIsUniqueGluip},
            {"LukeH", GetIsUniqueLukeH },
            {"Jamiec", GetIsUniqueJamiec },
            {"MrKWatkins", GetIsUniqueMrKWatkins }
        };

        var output = new StringBuilder();

        Console.WriteLine("Function,SetSize,DupeLocation,TotalTicks,AverageTicks");
        output.AppendLine("Function,SetSize,DupeLocation,TotalTicks,AverageTicks");

        foreach (SetSize size in Enum.GetValues(typeof(SetSize)))
        {
            var sizevalue = (int) size;
            foreach (DupeLocation location in Enum.GetValues(typeof(DupeLocation)))
            {
                var data = CreateTestData((int)size, location);
                foreach (string functionKey in functions.Keys)
                {
                    var ticks = RunSet(functions[functionKey], data, Iterations);
                    var avg = ticks / Iterations;
                    Console.WriteLine($"{functionKey},{sizevalue},{location},{ticks},{avg}");
                    output.AppendLine($"{functionKey},{sizevalue},{location},{ticks},{avg}");
                }
            }
        }

        File.WriteAllText("output.csv", output.ToString());
        Process.Start("output.csv");
    }

    static long RunSet<T>(Func<IEnumerable<T>, bool> getIsUnique, IEnumerable<T> values, int iterations)
    {
        var stopwatch = new Stopwatch();
        stopwatch.Start();
        for (var i = 0; i < iterations; i++)
        {
            getIsUnique.Invoke(values);
        }
        stopwatch.Stop();
        return stopwatch.ElapsedTicks;
    }

    static bool GetIsUniqueLukeH<T>(IEnumerable<T> values)
    {
        var set = new HashSet<T>();

        foreach (T item in values)
        {
            if (!set.Add(item))
                return false;
        }
        return true;
    }

    static bool GetIsUniqueGluip<T>(IEnumerable<T> values)
    {
        return values.Count() == values.Distinct().Count();
    }

    static bool GetIsUniqueJamiec<T>(IEnumerable<T> list)
    {
        var hs = new HashSet<T>();
        return list.All(hs.Add);
    }

    static bool GetIsUniqueMrKWatkins<T>(IEnumerable<T> values)
    {
        HashSet<T> hashSet = new HashSet<T>();
        foreach (var value in values)
        {
            if (hashSet.Contains(value))
            {
                return false;
            }
            hashSet.Add(value);
        }
        return true;
    }

    static int[] CreateTestData(int size, DupeLocation location)
    {
        var result = new int[size];
        Parallel.For(0, size, i => { result[i] = i; });
        return SetDupe(result, location);
    }

    static int[] SetDupe(int[] values, DupeLocation location)
    {
        switch (location)
        {
            case DupeLocation.Early:
                values[1] = values[0];
                break;
            case DupeLocation.Center:
                var midpoint = values.Length / 2;
                values[midpoint] = values[midpoint + 1];
                break;
            case DupeLocation.Late:
                values[values.Length - 1] = values[values.Length - 2];
                break;
            // case DupeLocation.None: // do nothing.
        }
        return values;
    }
}
}


1 commentaires

Pour mes propres fins, je souhaite vérifier l'unicité pour valider la contribution, donc presque chaque fois que le code est appelé, je m'attends à obtenir des ensembles qui ne contiennent aucun duplicate. Plus de 99,99% du temps, il n'y aurait aucun avantage de performance pour mon cas d'utilisation. En fait, je suis allé avec .Count () == .Distinct (). Comptez () avec un commentaire, car cela communique le plus clairement l'intention du code.