2009-02-12 10 views
3

Il doit sûrement y avoir de nombreuses façons d'optimiser le code suivant, où je dois essentiellement à faire en sorte que beaucoup de zones de texte ne sont pas vides et lire leurs valeurs:Comment optimiser ce code?

if (foo1.Text.Length != 0 & bar1.Text.Length != 0) 
{ 
    output.Text += myStrings[i] + "/" + foo1.Text + "/" + bar1.Text; 
} 

if (foo2.Text.Length != 0 & bar2.Text.Length != 0) 
{ 
    output.Text += myStrings[i] + "/" + foo2.Text + "/" + bar2.Text; 
} 

if (foo3.Text.Length != 0 & bar3.Text.Length != 0) 
{ 
    output.Text += myStrings[i] + "/" + foo3.Text + "/" + bar3.Text; 
} 

if (foo4.Text.Length != 0 & bar4.Text.Length != 0) 
{ 
    output.Text += myStrings[i] + "/" + foo4.Text + "/" + bar4.Text; 
} 

if (foo5.Text.Length != 0 & bar5.Text.Length != 0) 
{ 
    output.Text += myStrings[i] + "/" + foo5.Text + "/" + bar5.Text; 
} 

if (foo6.Text.Length != 0 & bar6.Text.Length != 0) 
    output.Text += myStrings[i] + "/" + foo6.Text + "/" + bar6.Text; 

if (foo7.Text.Length != 0 & bar7.Text.Length != 0) 
{ 
    output.Text += myStrings[i] + "/" + foo7.Text + "/" + bar7.Text; 
} 

if (foo8.Text.Length != 0 & bar8.Text.Length != 0) 
{ 
    output.Text += myStrings[i] + "/" + foo8.Text + "/" + bar8.Text; 
} 

if (foo9.Text.Length != 0 & bar9.Text.Length != 0) 
{ 
    output.Text += myStrings[i] + "/" + foo9.Text + "/" + bar9.Text; 
} 

if (foo10.Text.Length != 0 & bar10.Text.Length != 0) 
{ 
    output.Text += myStrings[i] + "/" + foo10.Text + "/" + bar10.Text; 
} 

Répondre

9

Je mettrais les éléments répétés dans les tableaux, puis boucle sur eux.

TextBox[] foos = new TextBox[] { foo1, foo2, foo3, /* etc */ }; 
TextBox[] bars = new TextBox[] { bar1, bar2, bar3, /* etc */ }; 

for (int i = 0; i <= 10; i++) 
    if (foos[i].Text.Length != 0 && bars[i].Text.Length != 0) 
     output.Text += myStrings[i] + "/" + foos[i].Text + bars[i].Text; 

Bien sûr, si les éléments sont vraiment nommés, vous pouvez remplir séquentiellement les tableaux en recherchant les commandes de la collection Controls du formulaire, avec le nom « foo » + Number.toString().

+0

Ceci est une très bonne solution! De plus, les éléments sont nommés en séquence, ce qui facilite le processus. Je vous remercie! – BeefTurkey

+1

En règle générale, je n'aime pas cette approche en raison de l'association implicite et non évidente entre fos et barres. Je suggère de rendre l'association explicite via une classe FooBar et de créer une liste pour une utilisation ultérieure. –

5

Je voudrais boucle un peu plus de la Contrôles collection sur laquelle ces TextBoxes résident, puis filtre sur TextBox uniquement et vous vérifiez et concattez.

Je conseille aussi fortement d'utiliser un StringBuilder au lieu de + =.

+0

+1 pour StringBuilder. La concaténation de chaînes dans la question serait l'inefficacité majeure ici, et dans la mesure où il semble être utilisé, il pourrait même être perceptible par l'utilisateur. –

+0

Oui, vous avez probablement raison sur StringBuilder. Actuellement, cela prend beaucoup de secondes avant que toutes les chaînes soient générées. Merci pour le conseil! – BeefTurkey

+0

Merci pour les heads-up, bonne chance! –

0

Pourriez-vous faire foo1-foo10 dans un tableau, foo [10], et la même chose pour la barre? Cela vous permettrait d'exprimer cela comme une simple boucle.

0

Serait-il possible de faire un WebControl qui a textboxes appelé foo et bar, et qui a une fonction comme:

if (foo.Text.Length != 0 & bar.Text.Length != 0) 
    return myStrings[i] + "/" + foo.Text + "/" + bar.Text; 
else 
    return string.Empty; 

Mettez dix de ceux sur votre page, puis utilisez:

output.Text = myControl1.FooBar + myControl2.FooBar + myControl3.FooBar + ... 

(Toujours un peu brouillon, mais pas tout à fait si répétitif.)

1

Ecrivez une fonction qui accepte les types de barres foo &. Passez tous les foo1 & bar1 à foo10 et bar10 à la fonction pour obtenir les valeurs. Vous pouvez également créer un tableau de foo et de barre et vous pouvez boucler et appeler la méthode pour obtenir la chaîne.

1
foreach (Control ctrl in Page.Controls) { 
     if (ctrl is TextBox) { 
      if (ctrl.Text.Length != 0) { 
       output.Text += myStrings[i] + "/" + ctrl.Text; 
      } 
     } 
} 

Non testé, mais devrait fonctionner. Avec ceci vos boîtes de texte pourraient être nommées n'importe quoi.

2

Il y a beaucoup de façons de refactoriser cela. La méthode que vous choisissez dépendra de votre scénario et de vos besoins particuliers.

  1. une fonction qui prend une foo et un bar en tant que paramètre et retourne la chaîne, puis agréger cette chaîne dans un constructeur de chaîne
  2. Mettez les foos et les barres dans des collections et de la boucle sur ces collections. Dans ce scénario, les tableaux seraient utiles et fourniraient un moyen d'indexer mutuellement les tableaux.
  3. Prenez l'étape 2 un peu plus loin et créez un nouveau FooBar de classe qui contient un truc et une barre ensemble. De cette façon vous pouvez créer une collection de FooBars et vous n'avez plus d'association implicite entre eux, maintenant c'est explicite et codifié.
  4. Prenez l'élément 3 un peu plus loin et reconnaissez que vous êtes en train d'agréger une chaîne. Si vous utilisez une version récente de C#, utilisez votre Map/Reduce dans LINQ (.Select(). Aggregate()) pour transformer vos FooBars en leurs chaînes correspondantes, puis agréger les chaînes en sortie.

Tout cela est juste le truc sur le dessus de ma tête. Si vous travaillez plus, je suis sûr que vous pouvez faire encore mieux.:)

(Dans ce travail à domicile, s'il vous plaît ajouter un tag devoirs.)

EDIT:

Je ne peux pas empêcher de se demander au sujet de la conception de l'interface utilisateur en général en fonction de votre commentaire dans un autre post indiquant qu'il faut "plusieurs secondes" pour concaténer les chaînes ensemble. 10 chaînes est une quantité de temps assez triviale, mais cela suggère que votre boucle externe (qui génère i) est assez longue.

Si vous êtes dans une position où vous avez la liberté de prendre de telles décisions, êtes-vous certain que votre interface utilisateur est réellement bonne pour la tâche à accomplir? Une grande collection de paires de zones de texte est une interface utilisateur difficile en général. Peut-être qu'un ListView serait plus approprié. Il contiendrait implicitement une collection, de sorte que vous n'auriez pas à faire face à cette bêtise «dix zones de texte» et serait une interface utilisateur plus facile à suivre pour vos utilisateurs en général.

+0

Je pense que vous pourriez avoir quelque chose à propos de ListView. En fait, je vais re-code pour implémenter un élément ListView au lieu d'un ensemble fixe de zones de texte. Merci pour le conseil. – BeefTurkey

Questions connexes