C# 4.0 - meilleure façon de refactoriser un bloc de "If (quelquechose est le type) {}" instructions?

Question

J'ai un code qui ressemble à ceci,

public void ResetControls(Control controlOnPage) 
{ 
    if (controlOnPage is TextBox) 
    { 
     ResetTextBoxControl(controlOnPage); 
    } 

    if (controlOnPage is MediaPicker) 
    { 
     ((MediaPicker)controlOnPage).Media = null; 
    } 

    if (controlOnPage is RelatedContentPicker) 
    { 
     ((RelatedContentPicker)controlOnPage).RelatedContentCollection = null; 
    } 
    ... 
    ... 

    foreach (Control child in controlOnPage.Controls) 
    { 
     ResetControls(child); 
    } 
} 

L'idée sous-jacente est que je peux passer d'une page à la méthode et réinitialiserons récursive tous les contrôles à leurs états par défaut - dans le cas de MediaPicker et RelatedContentPicker, ce sont des contrôles utilisateur que j'ai créés. FXCop m'avertit "Ne pas lancer inutilement" pour ce code - mais je ne sais pas comment le réécrire pour le rendre meilleur. Des idées?

Answer 1

Je pense que FXCop n'aime pas le code, car une bonne solution orientée objet serait d'ajouter la méthode virtuelle ResetControl à la classe Control (ce que vous ne pouvez évidemment pas faire). Si vous vouliez une solution orientée objet claire, vous pouvez créer une interface IResetableControl et créer une classe dérivée pour chacun des contrôles qui implémenteraient l'interface.

Si vous voulez juste faire le code existant syntaxiquement plus agréable, vous pouvez utiliser la méthode d'assistance suivante:

public void IfCast<T>(object obj, Action<T> f) { 
    if (obj is T) f((T)obj); 
} 

alors vous pouvez écrire:

IfCast(controlOnPage, (TextBox t) => 
    ResetTextBoxControl(t)); 

IfCast(controlOnPage, (MediaPicker mp) => { 
    mp.Media = null; }); 

Cela a exactement la même sémantique comme votre code original, mais c'est un peu plus sympa (et je pense que FxCop l'acceptera). Notez que le paramètre de type générique est déduit du type spécifié dans l'expression lambda, par ex. (TextBox t). Cela supprime également la nécessité d'effectuer une conversion supplémentaire dans le code qui gère le cas, car vous obtenez déjà une valeur du bon type (par exemple, mp).

Answer 2

Pour éviter la distribution dupliquée, vous pouvez transformer ceci:

if (controlOnPage is MediaPicker) 
{ 
    ((MediaPicker)controlOnPage).Media = null; 
} 

être:

MediaPicker mediaPickerControl = controlOnPage as MediaPicker; 
if (mediaPickerControl != null) 
{ 
    mediaPickerControl.Media = null; 
} 

Répétez l'opération pour vos autres moulages.

Vous pouvez également combiner avec certains cas imbriqués afin que vous ne devez pas essayer de faire le casting si vous le savez déjà quelque chose d'autre, comme une zone de texte

Answer 3

La chose la plus évidente à faire est de jeter un else devant les if « s:

if (controlOnPage is TextBox) 
{ 
    ResetTextBoxControl(controlOnPage); 
} 

else if (controlOnPage is MediaPicker) 
{ 
    ((MediaPicker)controlOnPage).Media = null; 
} 

else if (controlOnPage is RelatedContentPicker) 
{ 
    ((RelatedContentPicker)controlOnPage).RelatedContentCollection = null; 
} 

Il n'y a pas besoin d'essayer de jeter à MediaPicker ou RelatedContentPicker si le contrôle est un TextBox. Je les commanderais aussi de sorte que les cas les plus communs viennent avant les cas moins communs.