2010-08-04 5 views
3

désolé pour la question de débutant, mais je suis nouveau à la programmation ..Améliorer le code: Comparer 2 liste des éléments

Je veux vérifier s'il n'y a déjà plus d'un élément de TypeA dans listOfDifferentTypes. J'ai le code suivant:

 public void CheckType (Object param) 
    { 
      if (param is TypeA) 
      { 
       int i = 0; 
       TypeA paramToCheck = (TypeA) param; 

       foreach (var paramB in listOfDifferentTypes) 
       { 
        if (paramB is TypeA) 
        { 
         var paramInList = (TypeA) paramB; 
         if (paramToCheck.ID == paramInList.ID) 
         { 
          i++; 
         } 
        } 
       } 
       if (i > 1) 
       { 
        paramToCheck.m_Error = "ErrorText"; 
       }  
      } 
    } 

Je considère que ce n'est pas une solution très propre. Ce code peut-il être amélioré/optimisé?

Répondre

2

Vous pouvez utiliser LINQ pour cette :) Il sera agréable:

//Checks for string - resplace <string> with <some type> for other types 
private bool moreThanOne(List<object> differentTypes) 
{ 
    return differentTypes.OfType<string>().Count() > 1; 
} 

Utilisation:

List<object> listOfDifferentTypes = new List<object> { "string", 13, 52, "string", 54.3f }; 

var res = moreThanOne(listOfDifferentTypes); 

Je vois que vous vérifie également une sorte d'identité puis essayez ceci:

MISE À JOUR pour faire ce que votre code fait

Mise à jour: remplacé .Count() avec .Skip (1) .Tout(): il arrête si plus de 1 se trouve :)

private void CheckType(object param, List<object> differentTypes) 
{ 
    var paramToCheck = param as TypeA; 

    if (paramToCheck == null) return; 

    var res = differentTypes.OfType<TypeA>().Where(t => t.ID == paramToCheck.ID).Skip(1).Any(); 

    if (res) paramToCheck.m_Error = "error text"; 
} 

Comme je l'ai fait, vous pouvez remplacer:

if (param is TypeA) 
{ 
    TypeA paramToCheck = (TypeA) param; 
    ... Do something 

Avec:

TypeA paramToCheck = param as TypeA; //Returns null if not a TypeA 
if (param == null) return; 

... Do something 

Il est un peu plus vite :)

+0

Merci, LINQ semble très cool. Je dois envisager de l'utiliser) – user410570

1

Votre solution originale, réécrite:

public void CheckType(Object param) 
{ 
    TypeA paramToCheck = param as TypeA; 
    int count = 0; 
    if (paramToCheck != null) 
    { 
     foreach (var paramB in listOfDifferentTypes) 
     { 
      var paramInList = paramB as TypeA; 
      if (paramInList != null && paramToCheck.ID == paramInList.ID) 
      { 
       count++; 

       if (count > 1) 
       { 
        paramToCheck.m_Error = "ErrorText"; 
        break; 
       } 
      } 
     } 
    } 

}

Notes:

  • utiliser le mot-clé as une comparaison contre null pour effectuer la conversion de type,
  • combinent plusieurs conditions dans une seule instruction if (en utilisant la ET (&&) opérateur),
  • utilisez le break instruction de quitter la boucle foreach dès que votre condition a été satisfaite.
  • Ceci est juste une version nettoyée de votre code original; il n'y a pas de doute de meilleures façons d'atteindre votre comportement désiré :-)

Modifier: mise à jour: commentaires (re! Merci pour remarquer mon erreur précédente)

+0

Hhm ce n'est pas exactement pareil? Votre code écrirait une erreur s'il y a une ou plusieurs instances. Son code écrirait l'erreur s'il y a PLUS D'UNE instance. Corrigez-moi si je me trompe :) –

+0

Mon mauvais - code mis à jour! – robyaw

Questions connexes