Modèle pour une classe qui ne fait qu'une seule chose

Disons que j'ai une procédure qui fait des trucs :

void doStuff(initalParams) {
    ...
}

Maintenant, je découvre que "faire des choses" est une opération assez complexe. La procédure devient volumineuse, je l'ai divisée en plusieurs procédures plus petites et bientôt je me rends compte qu'avoir une sorte d' état serait utile tout en faisant des trucs, de sorte que j'ai besoin de passer moins de paramètres entre les petites procédures. Donc, je le factorise dans sa propre classe:

class StuffDoer {
    private someInternalState;

    public Start(initalParams) {
        ...
    }

    // some private helper procedures here
    ...
}

Et puis je l'appelle comme ça:

new StuffDoer().Start(initialParams);

ou comme ça:

new StuffDoer(initialParams).Start();

Et c'est ce qui ne va pas. Lorsque j'utilise l'API .NET ou Java, je n'appelle jamais new SomeApiClass().Start(...);, ce qui me fait penser que je me trompe. Bien sûr, je pourrais rendre le constructeur de StuffDoer privé et ajouter une méthode d'assistance statique:

public static DoStuff(initalParams) {
    new StuffDoer().Start(initialParams);
}

Mais alors j'aurais une classe dont l'interface externe se compose d'une seule méthode statique, ce qui semble également bizarre.

D'où ma question: existe-t-il un modèle bien établi pour ce type de classes qui

• avoir un seul point d'entrée et
• n'ont pas d'état "reconnaissable en externe", c'est-à-dire que l'état d'instance n'est requis que pendant l' exécution de ce point d'entrée?

Il existe un modèle appelé objet de méthode dans lequel vous factorisez une seule grande méthode avec beaucoup de variables / arguments temporaires dans une classe distincte. Vous faites cela plutôt que d'extraire simplement des parties de la méthode dans des méthodes distinctes car elles ont besoin d'accéder à l'état local (les paramètres et les variables temporaires) et vous ne pouvez pas partager l'état local à l'aide de variables d'instance car elles seraient locales à cette méthode (et les méthodes qui en sont extraites) uniquement et ne seraient pas utilisées par le reste de l'objet.

Ainsi, à la place, la méthode devient sa propre classe, les paramètres et les variables temporaires deviennent des variables d'instance de cette nouvelle classe, puis la méthode est divisée en méthodes plus petites de la nouvelle classe. La classe résultante a généralement une seule méthode d'instance publique qui exécute la tâche que la classe encapsule.

Je dirais que la classe en question (utilisant un seul point d'entrée) est bien conçue. Il est facile à utiliser et à étendre. Assez SOLIDE.

Même chose pour no "externally recognizable" state. C'est une bonne encapsulation.

Je ne vois aucun problème avec du code comme:

var doer = new StuffDoer(initialParams);
var result = doer.Calculate(extraParams);

Du point de vue des principes SOLIDES , la réponse de Jgauffin est logique. Cependant, vous ne devez pas oublier les principes généraux de conception, par exemple le masquage d'informations .

Je vois plusieurs problèmes avec l'approche donnée:

• Comme vous l'avez souligné vous-même, les gens ne s'attendent pas à utiliser le mot-clé «nouveau», lorsque l'objet créé ne gère aucun état . Votre conception reflète son intention. Les utilisateurs de votre classe peuvent être confus quant à l'état dans lequel ils sont gérés et si les appels ultérieurs à la méthode peuvent entraîner un comportement différent.
• Du point de vue de la personne qui utilise la classe, l'état intérieur est bien caché, mais lorsque vous voulez apporter des modifications à la classe ou simplement la comprendre, vous complexifiez les choses. J'ai déjà beaucoup écrit sur les problèmes que je vois avec les méthodes de fractionnement juste pour les rendre plus petites , en particulier lors du déplacement de l'état dans la portée de la classe. Vous modifiez la façon dont votre API doit être utilisée juste pour avoir des fonctions plus petites! À mon avis, cela va certainement trop loin.

Quelques références liées

Peut - être un point essentiel de l' argumentation réside dans la façon dont la mesure d'étirer la responsabilité unique principe . "Si vous allez à l'extrême et que vous construisez des classes qui ont une raison d'exister, vous pouvez vous retrouver avec une seule méthode par classe. Cela entraînerait une grande prolifération de classes pour les processus les plus simples, ce qui entraînerait difficile à comprendre et difficile à changer. "

Une autre référence pertinente concernant ce sujet: "Divisez vos programmes en méthodes qui effectuent une tâche identifiable. Conservez toutes les opérations d'une méthode au même niveau d'abstraction ." - Kent Beck Key est ici "le même niveau d'abstraction". Cela ne signifie pas «une chose», comme cela est souvent interprété. Ce niveau d'abstraction dépend entièrement du contexte pour lequel vous concevez.

Quelle est donc la bonne approche?

Sans connaître votre cas concret d'utilisation, c'est difficile à dire. Il y a un scénario dans lequel j'utilise parfois (pas souvent) une approche similaire. Lorsque je souhaite traiter un ensemble de données, sans vouloir rendre cette fonctionnalité disponible pour l'ensemble de la portée de la classe. J'ai écrit un blog à ce sujet, comment les lambdas peuvent encore améliorer l'encapsulation . J'ai également commencé une question sur le sujet ici sur les programmeurs . Ce qui suit est un dernier exemple où j'ai utilisé cette technique.

new TupleList<Key, int>
{
    { Key.NumPad1, 1 },
            ...
    { Key.NumPad3, 16 },
    { Key.NumPad4, 17 },
}
    .ForEach( t =>
    {
        var trigger = new IC.Trigger.EventTrigger(
                        new KeyInputCondition( t.Item1, KeyInputCondition.KeyState.Down ) );
        trigger.ConditionsMet += () => AddMarker( t.Item2 );
        _inputController.AddTrigger( trigger );
    } );

Étant donné que le code très «local» dans le ForEachn'est réutilisé nulle part ailleurs, je peux simplement le conserver à l'emplacement exact où il est pertinent. Décrire le code de telle manière que le code qui dépend les uns des autres est fortement regroupé le rend plus lisible à mon avis.

Alternatives possibles

• En C #, vous pouvez utiliser des méthodes d'extension à la place. Opérez donc directement sur l'argument que vous passez à cette méthode «une chose».
• Vérifiez si cette fonction n'appartient pas à une autre classe.
• Faites-en une fonction statique dans une classe statique . C'est probablement l'approche la plus appropriée, comme le reflètent également les API générales que vous avez mentionnées.

Je dirais que c'est assez bon, mais que votre API devrait toujours être une méthode statique sur une classe statique, car c'est ce que l'utilisateur attend. Le fait que la méthode statique utilise newpour créer votre objet d'assistance et effectuer le travail est un détail d'implémentation qui doit être caché à celui qui l'appelle.

new StuffDoer().Start(initialParams);

Il y a un problème avec les développeurs désemparés qui pourraient utiliser une instance partagée et l'utiliser

1. plusieurs fois (ok si l'exécution précédente (peut-être partielle) ne gâche pas l'exécution ultérieure)
2. à partir de plusieurs threads (pas OK, vous avez explicitement dit qu'il a un état interne).

Cela nécessite donc une documentation explicite selon laquelle il n'est pas sûr pour les threads et s'il est léger (sa création est rapide, ne bloque pas les ressources externes ni beaucoup de mémoire) qu'il est OK d'instancier à la volée.

Le masquer dans une méthode statique aide à cela, car la réutilisation de l'instance ne se produit jamais.

S'il a une initialisation coûteuse, il pourrait (ce n'est pas toujours) avantageux de préparer l'initialisation une fois et de l'utiliser plusieurs fois en créant une autre classe qui contiendrait uniquement l'état et la rendrait clonable. L'initialisation créerait un état dans lequel serait stocké doer. start()le clonerait et le transmettrait à des méthodes internes.

Cela permettrait également d'autres choses comme l'état persistant d'exécution partielle. (Si cela prend du temps et que des facteurs externes, par exemple une panne de courant, peuvent interrompre l'exécution.) Mais ces éléments fantaisistes supplémentaires ne sont généralement pas nécessaires, donc cela n'en vaut pas la peine.

Outre mon autre réponse plus élaborée et personnalisée , je pense que l'observation suivante mérite une réponse distincte.

Il y a des indications que vous suivez peut-être l'anti-modèle de Poltergeists .

Les poltergeists sont des classes avec des rôles très limités et des cycles de vie efficaces. Ils démarrent souvent des processus pour d'autres objets. La solution refactorisée comprend une réaffectation des responsabilités à des objets à vie plus longue qui éliminent les Poltergeists.

Symptômes et conséquences

• Chemins de navigation redondants.
• Associations transitoires.
• Classes sans état.
• Objets et classes temporaires de courte durée.
• Classes à opération unique qui existent uniquement pour «amorcer» ou «invoquer» d'autres classes via des associations temporaires.
• Classes avec des noms d'opérations «de type contrôle» comme start_process_alpha.

Solution refactorisée

Les Ghostbusters résolvent les Poltergeists en les supprimant complètement de la hiérarchie des classes. Après leur suppression, cependant, la fonctionnalité qui était «fournie» par le poltergeist doit être remplacée. C'est facile avec un simple réglage pour corriger l'architecture.

Il existe plusieurs modèles sur le catalogue P de EAA de Martin Fowler ;

1. Script de transaction : quelque peu similaire à l'approche que vous avez mentionnée.
2. Remplacer la méthode avec l'objet Méthode : ressemble également à une bonne approche, comme l'a mentionné @anon.