7
votes

Vous cherchez une meilleure façon de trier ma liste

J'examine un morceau de code que j'ai écrit il n'y a pas trop longtemps, et je déteste simplement la façon dont j'ai géré le tri - je me demande si quelqu'un pourrait me montrer une meilleure façon.

J'ai Une classe, Holding , qui contient des informations. J'ai une autre classe, HoldingsList , qui contient une liste membre. J'ai aussi une énumération, portfoliosheetMapping , qui a ~ 40 éléments ou donc des éléments.

Il ressemble à ceci: xxx < p> J'ai une méthode qui peut appeler la liste à trier en fonction de laquelle l'énumération sélectionne l'utilisateur. La méthode utilise un énoncé de commutateur Mondo qui compte plus de 40 cas (UGH!).

Un extrait court ci-dessous illustre le code: xxx

mon problème est avec l'instruction de commutation. Le commutateur est étroitement lié au portfoliosheetMapping Enum, qui peut changer demain ou le lendemain. Chaque fois qu'il change, je vais devoir revenir sur cette instruction de commutation et ajouter une autre affaire en bloc. Je suis effrayé que finalement que cette déclaration de commutation augmente si grosse qu'elle soit totalement ingérable.

Peut-être que quelqu'un peut me dire s'il y a une meilleure façon de trier ma liste?


7 commentaires

Pourquoi ce tri est-il fait? Est-ce seulement à des fins d'affichage?


@Hans, l'instance de classe est désérialisée à partir d'une feuille de calcul Excel contenant des données d'analyse de portefeuille. L'action réelle est appelée à partir d'un bouton de la barre d'outils Excel (mais c'est vraiment non pertinent), et une fois le tri terminé, je redésiomise les objets à la feuille de calcul. Le tri affecte réellement divers autres éléments de la feuille de calcul Excel, il n'est donc pas purement d'affichage uniquement.


S'étendant sur mon commentaire sur la réponse de Mark ... Serait-il possible que vous puissiez refactoriser votre code pour gérer les données dans un formulaire de type de base de données? Vous n'avez peut-être même pas besoin de l'énumé si vous avez utilisé un format de style plus de base de données (comme vous pouvez simplement ajouter et supprimer les colonnes de votre "table" au besoin), donc si vous êtes prêt à mettre dans le temps / effort à faire Ce refactoring, vous pourriez vous retrouver avec quelque chose de plus élégant, afin de parler. Bien sûr, il peut y avoir des aspects de votre programme ailleurs qui pourraient faire une telle approche moins réalisable qu'il ne semble ...


@ Code4Life: Ce n'est pas vraiment lié à votre question, mais si vous travaillez avec une feuille de calcul Excel, pourquoi avez-vous besoin d'extraire les données de cela comme ça? Êtes-vous incapable d'utiliser COM pour manipuler directement les données?


@Jab, eh bien les données proviennent d'une feuille de calcul d'analyse de portefeuille (Excel), donc dans la présente partie du cadre, la plupart des classes sont orientées vers la fonctionnalité de la feuille de calcul. Cela prendrait un refactoring important pour changer cela ... Je n'ai pas senti que cela était pertinent cependant, alors j'ai omis que cela indique le début.


@Jab, j'utilise vsto. L'utilisation de COM est une sorte de lent, donc je retire les gammes dont j'ai besoin d'un objet [] pour la manipulation de données. Les classes que j'ai sont plus ou moins façons pour le tableau, qui sont ensuite repoussées dans la feuille de calcul correspondante / gammes sur le classeur cible.


@ Code4Life: Ah, je vois. Ça a du sens.


7 Réponses :


3
votes

Avez-vous regardé dans Dynamic Linq

Plus précisément, vous pouvez simplement faire quelque chose comme: xxx

note / Code> Propriété La première fois. Dans ces cas, Dynamiclinq aura besoin de voir, par exemple, "produit.productd" . Vous pouvez refléter le nom de la propriété ou simplement utiliser une valeur «bien connue» et une concurrence avec l'ENUM .tostring () . À ce stade, je force ma réponse à votre question afin que cela soit au moins une solution de travail.


6 commentaires

C'est une mauvaise réponse que je pense, on dirait "Lisez ceci, peut-être que cela aide et je suis trop paresseux pour vous aider à trouver une solution" =)


@Marc, merci pour le lien (pas de jeu de mots), mais comment cela aiderait-il à réduire mon énorme relevé de commutation?


Wow, je vais éditer mon message pour mettre quelque chose de spécifique et les déclencheurs itinérants sortent. Le ciel nous interdit que je suis un peuple lent de Typper! ;)


@Restuta: Code4Life travaille avec des données qui semble que cela semble être traité le plus efficacement dans une représentation de la base de données (la partie qui me démarquait était le fait que les données ont un ensemble de colonnes nommées, chacune peut être la colonne de tri. pour les données). Marc a posté un lien qui explique comment faire de telles requêtes de manière dynamique, ce que veut Code4Life.


@Marc, je pense que je n'étais pas clair. Pf.holdings est une liste et C.Product référencent réellement une instance de produit. Donc, un simple ordre des ordres (string) ne fonctionnera pas, afaik ... désolé pour ça - je vais mettre à jour mon message.


@Jab sans explication sa réponse n'est pas utile, cela est prouvé par le fait qu'il a modifié sa réponse.



1
votes

Vous pouvez implémenter une classe Icomarer personnalisée qui utilise la réflexion. Cependant, cela serait plus lent.

Voici une classe que j'ai utilisée une fois: p> xxx pré>

puis utilisez-le comme ceci: p>

var comparer = new ListComparer() { 
    Field= frm.SelectedSortColumn.BaseColumn.ToString() };
if (frm.SortAscending)
    pf.Holding = pf.Holding.OrderBy(h=>h.Product, comparer).ToList();
else
    pf.Holding = pf.Holding.OrderByDescending(h=>h.Product, comparer).ToList();


2 commentaires

Je suis prêt à regarder cela - pouvez-vous élaborer un peu plus sur la façon dont la réflexion serait utilisée? Je ne sais pas comment cela correspond à mon énoncé de commutation et aux valeurs d'énumération ...


Toutefois, si votre version-cadre le permet, je préférerais la solution dynamique LINQ!



1
votes

Que diriez-vous:

Func<Holding, object> sortBy;

switch (frm.SelectedSortColumn.BaseColumn)
{
    case PortfolioSheetMapping.IssueId:
        sortBy = c => c.Product.IssueId;
        break;
    case PortfolioSheetMapping.MarketId:
        sortBy = c => c.Product.MarketId;
        break;
    /// etc.
}

/// EDIT: can't use var here or it'll try to use IQueryable<> which doesn't Reverse() properly
IEnumerable<Holding> sorted = pf.Holdings.OrderBy(sortBy);
if (!frm.SortAscending)
{
    sorted = sorted.Reverse();
}


3 commentaires

@Michael corrige l'original où j'avais copié le mal si, c'est correct maintenant oui?


C'est correct, mais je préférerais iEnumerable trié = (frm.sorcending)? pf.holdings.orderby (sortby): pf.holdings.orderbydescendances (sortby). Cela évite l'inverse ()


@Michael Je ne suis pas sûr que cela fait une énorme différence, il semble que les ordres-médicaments-médicaments reviennent simplement un énumérateur inversé en interne de toute façon à en juger par la sortie du réflecteur, mais je reçois votre point de vue.



4
votes

Vous pouvez essayer de réduire le commutateur à quelque chose comme ceci:

    private static readonly Dictionary<PortfolioSheetMapping, Func<Holding, object>> sortingOperations = new Dictionary<PortfolioSheetMapping, Func<Holding, object>>
    {
        {PortfolioSheetMapping.Symbol, h => h.Symbol},
        {PortfolioSheetMapping.Quantitiy, h => h.Quantitiy},
        // more....
    };

    public static List<Holding> SortHoldings(this List<Holding> holdings, SortOrder sortOrder, PortfolioSheetMapping sortField)
    {
        if (sortOrder == SortOrder.Decreasing)
        {
            return holdings.OrderByDescending(sortingOperations[sortField]).ToList();
        }
        else
        {
            return holdings.OrderBy(sortingOperations[sortField]).ToList();                
        }
    }


2 commentaires

J'étais sur le point de suggérer un dictionnaire de classes de comparaison, mais c'est beaucoup Natre!


Merci pour le conseil! Ceci est certainement plus facile à coder, mais je pense que je vais aller avec la suggestion de @ Lukeh à utiliser .sort () au lieu de cela, car c'est un peu plus rapide. Mais conceptuellement la même approche que vous (à l'aide d'un dictionnaire, etc., etc.).



1
votes

Il me semble qu'il y a deux améliorations immédiates que nous pouvons faire:

  • la logique qui utilise frm.sortascendend pour décider entre commanderby et OrderByDescenscends est dupliqué dans chaque case , et peut être retiré après le commutateur si le boîtier S est modifié pour ne rien faire d'établissement de la clé de tri et le mettre dans un Func

  • qui laisse toujours le commutateur bien sûr - et cela peut être remplacé par une carte statique (dans un dictionnaire , dites) de portfoliosheetcapping à un FUNC Prenant un Tenir et renvoie la touche de tri. par exemple


1 commentaires

Merci, ce sont de bonnes suggestions, aide définitivement à mieux moduler le code.



5
votes

Vous ré-attribuez les données triées directement à votre propriété pf.holdings , alors pourquoi ne pas contourner la surcharge de Orderby et Tolist et utilisez simplement la liste Trier Méthode directement à la place?

Vous pouvez utiliser une carte pour contenir Comparaison délégués pour toutes les tri prises en charge, puis appelez Trier (comparaison ) avec le délégué approprié: xxx


3 commentaires

Bonne suggestion, vaut vraiment la peine d'être essayée.


Merci! Anecdotage le .sort est légèrement mais définitivement plus rapide que .orderby (). Tolist ().


Nice, je me demandais comment taper ma solution, c'est mieux: le garder encapsulé dans le délégué.



1
votes

Si les propriétés de la classe de maintien (symbole, prix, etc.) sont le même type que vous pouvez effectuer les éléments suivants: xxx

qui affiche ensuite:

Valeurs d'origine:
Quantité = 2, prix = 5
Quantité = 7, prix = 2
Quantité = 1, prix = 3

Valeurs triés par prix:
Quantité = 7, prix = 2
Quantité = 1, prix = 3
Quantité = 2, prix = 5


2 commentaires

J'aime l'idée de func. Les propriétés de la classe de détention sont des types différents. Je devrais donc adopter une approche modifiée de ce que vous suggérez, mais une bonne idée néanmoins.


Je viens de voir les autres réponses postées lorsque je faisais le mien et que vous pouvez changer la recherche à la recherche = nouveau dictionnaire > et cela devrait fonctionner