2010-08-26 3 views
3

J'ai cette méthode appelée LoadToDictionary. Il accepte un dictionnaire et un chemin de fichier. Actuellement, il ressemblerait à ceci: void LoadToDictionary(Dictionary<string, string> dict, string filePath. Puis à l'intérieur, il aurait beaucoup de code pour extraire le contenu du fichier dans filePath vers un dictionnaire. Je veux le rendre plus général, de sorte qu'il pourrait, disons, prendre un dictionnaire avec int à la place, et la possibilité de changer le code d'analyse dans la méthode. Dois-je marquer cette méthode et la remplacer par la suite? Est-ce qu'il y a un autre moyen?Comment rendre cette méthode plus générale?

+0

S'il vous plaît, ajoutez le code afin que nous ayons une meilleure idée de ce qui peut être amélioré ... –

Répondre

36

La généralité prématurée donne une optimisation prématurée, une course à l'argent comme la racine du mal. Généralité est cher et un code coûteux devrait être justifié par des avantages évidents.

Je voudrais donc conduire la généralité accrue dans votre méthode sur la base de scénarios spécifiques. Je peux penser à plusieurs façons de rendre votre méthode plus générale:

  1. Ne prenez pas un dictionnaire de chaîne à chaîne; prendre un dictionnaire de, disons, une chaîne à un type arbitraire.

  2. Ne pas prendre un Dictionary<...>; Prenez un IDictionary<...>.

  3. Ne prenez aucune sorte de dictionnaire. Prenez un Action<K, V> dont l'action pourrait être d'entrer l'élément dans un dictionnaire.

  4. Ne prenez pas de nom de fichier. Vous allez tout simplement transformer le nom du fichier en un flux, alors prenez un Stream pour commencer. Demandez à l'appelant d'ouvrir le flux pour vous.

  5. Ne prenez pas de flux. Vous allez tout simplement transformer le flux en une séquence d'éléments, alors prenez un IEnumerable<...> et demandez à l'appelant d'analyser le flux pour vous.

Comment général pouvons-nous faire cela? Supposons que nous ayons une séquence de T qui est ensuite transformée en une carte de K à V. De quoi avons-nous besoin? Une séquence, un extracteur clé, un extracteur de valeur et un écrivain carte:

static void MakeMap<T, K, V>(this IEnumerable<T> sequence, Action<K, V> mapWriter, Func<T, K> keyExtractor, Func<T, V> valueExtractor) 
{ 
    foreach(var item in sequence) 
     mapWriter(keyExtractor(item), valueExtractor(item)); 
} 

Remarquez comment le code devient vraiment court quand il est vraiment général. C'est assez bizarre en général, et par conséquent le site d'appel ressemblera maintenant à un désordre impie. Est-ce vraiment ce que tu veux? Allez-vous utiliser toute cette généralité efficacement? Probablement pas. Quels sont les scénarios que vous avez réellement qui poussent le désir d'être plus général? Utilisez ces scénarios spécifiques pour motiver votre refactoring.

Est-ce aussi loin que nous pouvons aller? Pourrions-nous être encore plus général que cela? Sûr. Nous pourrions remarquer par exemple qu'il semble mauvais d'avoir un effet secondaire là-dedans. La méthode ne devrait-elle pas renvoyer un dictionnaire nouvellement construit? Quelle devrait être la politique de comparaison de ce dictionnaire? Que faire s'il y a plus d'un article qui a la même clé? Devrait-il être remplacé, ou devrions-nous avoir le dictionnaire en fait une carte des clés à une liste de valeurs? Nous pourrions prendre ces idées et de faire une nouvelle méthode qui permet de résoudre tous ces problèmes:

public static ILookup<TKey, TElement> ToLookup<TSource, TKey, TElement>(
this IEnumerable<TSource> source, 
Func<TSource, TKey> keySelector, 
Func<TSource, TElement> elementSelector, 
IEqualityComparer<TKey> comparer) { ... } 

Et puis, nous avons juste réinventé la méthode d'extension ToLookup(). Parfois, quand vous faites une méthode assez générale, vous découvrez qu'elle existe déjà.

+0

Je dois dire que vous avez raison. Vous avez donné de très bonnes idées ici. Je vais probablement repenser à ça. – DMan

+3

+1 de ma part pour une réponse * géniale *. Mon seul ajout (purement une opinion) serait que si vous avez un morceau de code que vous pouvez rendre plus générique et réutilisable, vous pouvez simplifier votre code en ayant quelques méthodes de "pass-through" qui l'appellent détails, comme le fait qu'il fonctionne sur un 'IDictionary ' de sorte que votre code d'appel ne devienne pas phénoménalement complexe comme Eric l'a mentionné. – Rob

+1

Réponse impressionnante en effet. Je suis entièrement d'accord. C'est le principe de YAGNI en action. –

1

Je vois deux vraies questions ici:
1) Une signature de méthode peut-elle être dynamique, de sorte que les types attendus peuvent changer?
2) Un corps de méthode peut-il être dynamique, de sorte que ma logique puisse changer à la volée?

Je ne suis pas sûr comment l'un ou l'autre de ces deux pourrait être accompli sans utiliser Reflection et beaucoup de code gen. Je suis sûr qu'il y a ceux qui sont ici, cependant. En fonction de ce dont vous avez besoin, cependant, la surcharge de la méthode de base ressemble à ce que vous cherchez réellement.

En utilisant votre exemple:

void LoadToDictionary(Dictionary<string, string> dict, string filePath) 
{ ... code here ... } 

//to have a LoadToDictionary method which accepts Dictionary<int, int> 
void LoadToDictionary(Dictionary<int, int> dict, string filePath) 

//To change the processing logic, you either need a new method name, 
// or to override the original in an inheriting class 

void AlternateLoadToDictionary(Dictionary<string, string> dict, string filePath) 
override void LoadToDictionary(Dictionary<string, string> dict, string filePath) 

Une alternative encore meilleure est d'avoir votre logique d'analyse séparée de votre logique de création de dictionnaire. Vous pouvez même appeler l'autre méthode à l'intérieur de votre méthode LoadDictionary donc normale surcharge pourrait travailler là aussi

void LoadToDictionary(Dictionary<string, string> dict, string filePath) 
{ 
    string[] fileLines; 

    if([yourBaseLineCondition]) 
    fileLines = LoadDataFromFile(filePath, false); 

    else fileLines = LoadDataFromFile(filePath, true); 
} 

string[] LoadDataFromFile(string filePath, bool altLogic) 
{ 
    if(altLogic) return LoadDataFromFileAlt(filePath) 
    else 
    { ... your logic ... } 
} 

string[] LoadDataFromFileAlt(string filePath) 
{ ... your alt logic... } 

méthode surchargeant de base vous donne beaucoup de flexability. Plus important encore, il vous permet de suivre le principe de YAGNI sans pour autant couper les pistes d'expansion. Vous pouvez toujours revenir dans votre code et ajouter la méthode LoadToDictionary<myCustomObject, myCustomObject> si vous avez besoin de le faire.

Questions connexes