Est-il acceptable de copier et coller du code long mais simple au lieu de les encapsuler dans une classe ou une fonction?

29

Supposons que je dispose d'un segment de code pour me connecter à Internet et afficher des résultats de connexion similaires:

HttpRequest* httpRequest=new HttpRequest();
httpRequest->setUrl("(some domain .com)");
httpRequest->setRequestType(HttpRequest::Type::POST);
httpRequest->setRequestData("(something like name=?&age=30&...)");
httpRequest->setResponseCallback([=](HttpClient* client, HttpResponse* response){
    string responseString=response->getResponseDataString();
        if(response->getErrorCode()!=200){
            if(response->getErrorCode()==404){
                Alert* alert=new Alert();
                alert->setFontSize(30);
                alert->setFontColor(255,255,255);
                alert->setPosition(Screen.MIDDLE);
                alert->show("Connection Error","Not Found");
            }else if((some other different cases)){
                (some other alert)
            }else
                Alert* alert=new Alert();
                alert->setFontSize(30);
                alert->setPosition(Screen.MIDDLE);
                alert->setFontColor(255,255,255);
                alert->show("Connection Error","unknown error");
            }
        }else{
            (other handle methods depend on different URL)
        }
}

le code est long et il est couramment utilisé, mais le code ci-dessus ne nécessite pas de choses supplémentaires telles que la fonction et la classe personnalisées (HttpRequest et Alert sont tous deux fournis par framework par défaut), et bien que le segment de code soit long, il est simple et pas complexe (il est long simplement parce qu'il y a des ensembles de paramètres tels que l'URL, la taille de la police ...), et le segment de code a peu de variations entre les classes (par exemple: url, demande de données, cas de gestion de code d'erreur, poignée normale cas ...)

Ma question est, est-il acceptable de copier et coller du code long mais simple au lieu de les envelopper dans une fonction pour réduire la dépendance du code?

ggrr
la source
89
Imaginez que vous ayez un bogue dans ce code, comme ne pas libérer les objets que vous allouez. (Votre framework libère-t-il les Alertobjets?) Imaginez maintenant que vous devez trouver chaque instance copiée de ce code pour corriger le bogue. Imaginez maintenant que ce n'est pas vous qui devez le faire, mais un tueur à la hache fou qui sait que vous êtes celui qui a créé toutes ces copies en premier lieu.
Sebastian Redl
8
Et BTW, mélanger la mise en réseau et l'affichage des erreurs en un seul endroit est déjà un gros non-non, à mon humble avis.
sleske
11
Non jamais. Entièrement inacceptable. Si vous étiez sur mon projet, vous ne seriez plus sur mon projet et vous seriez sur un programme de formation ou PIP.
nhgrif
10
Et vient le superviseur qui dit "Cette boîte d'alerte au milieu de mon écran est une terreur absolue. Je regarde des jifs de chats et la fenêtre pop-up bloque ma vue à chaque fois qu'elle apparaît. Veuillez la déplacer vers le haut à droite . " 3 semaines plus tard "Que diable avez-vous fait?! Je ne peux plus fermer mes jifs de chat parce que VOTRE pop-up couvre le X en haut à droite, corrigez-le."
MonkeyZeus
11
Je pense que tout le monde ici semble penser que c'est une mauvaise idée. Mais pour inverser la question, pourquoi ne mettriez-vous PAS ce code dans une classe ou une fonction distincte?
Karl Gjertsen

Réponses:

87

Vous devez considérer le coût du changement. Et si vous vouliez changer la façon dont les connexions sont établies? Serait-ce facile? Si vous avez beaucoup de code en double, trouver tous les endroits qui doivent être modifiés peut prendre beaucoup de temps et entraîner des erreurs.

Vous devez également considérer la clarté. Très probablement, avoir à regarder 30 lignes de code ne sera pas aussi facile à comprendre qu'un simple appel à une fonction "connectToInternet". Combien de temps va-t-on perdre à essayer de comprendre le code lorsque de nouvelles fonctionnalités doivent être ajoutées?

Il existe de rares cas où la duplication n'est pas un problème. Par exemple, si vous faites une expérience et que le code sera jeté à la fin de la journée. Mais en général, le coût de la duplication l'emporte sur les petites économies de temps de ne pas avoir à extraire le code dans une fonction distincte .

Voir également https://softwareengineering.stackexchange.com/a/103235/63172

Vaughn Cato
la source
19
... et qui sait si ces 30 lignes sont vraiment les mêmes que celles que vous avez consultées précédemment ou si quelqu'un a changé de port ou d'adresse IP dans sa copie pour une raison quelconque. L'hypothèse implicite que " tout groupe de quelque 30 lignes commençant par le HttpRequestsont tous les mêmes " est une erreur facile à faire.
null
@null Excellent point. J'ai une fois travaillé sur du code où il y avait des connexions de connexion à la base de données copiées et collées partout, mais certaines avaient de subtiles différences dans la configuration. Je n'avais aucune idée s'il s'agissait de changements importants, délibérés ou simplement de différences aléatoires
user949300
54

Non.

En fait, même votre code "simple" devrait être divisé en parties plus petites. Au moins deux.

Un pour établir la connexion et gérer la réponse normale de 200. Par exemple, que se passe-t-il si vous passez d'un POST à ​​un PUT dans certains cas? Que se passe-t-il si vous effectuez des millions de ces connexions et avez besoin d'un multi-threading ou d'un pool de connexions? Avoir le code en un seul endroit, avec un argument pour la méthode, rendra cela beaucoup plus facile

De même, un autre pour gérer les erreurs. Par exemple, si vous modifiez la couleur ou la taille de police de l'alerte. Ou vous rencontrez des problèmes avec les connexions intermittentes et souhaitez enregistrer les erreurs.

user949300
la source
Vous pouvez également citer SRP: avoir un bloc de code qui n'a qu'un seul but le rend beaucoup plus facile à comprendre et à maintenir ..
Roland Tepp
C'est un cas où DRY et SRP s'alignent réellement. Parfois non.
user949300
18

est-il acceptable de copier et coller ...

Non.

Pour moi, l'argument décisif est celui-ci:

... il est couramment utilisé ...

Si vous utilisez un morceau de code à plus d'un endroit, alors, quand il change, vous devez le changer à plus d'un endroit ou vous commencez à obtenir des incohérences - des "choses étranges" commencent à se produire (c'est-à-dire que vous introduisez des bogues).

c'est simple et pas complexe ...

Et il devrait être d'autant plus facile de refactoriser une fonction.

... il existe des ensembles de paramètres tels que l'URL, la taille de la police ...

Et qu'est-ce que les utilisateurs aiment changer? Polices, tailles de police, couleurs, etc., etc.

À présent; à combien d'endroits devrez-vous changer ce même morceau de code pour leur redonner la même couleur / police / taille? (Réponse recommandée: une seule ).

... le segment de code a peu de variations entre les classes (par exemple: URL, données de demande, cas de traitement de code d'erreur, cas de traitement normaux ...)

Variation => paramètre (s) de fonction.

Phill W.
la source
"Et qu'est-ce que les utilisateurs aiment changer? Polices, tailles de police, couleurs, etc." Voulez-vous vraiment changer cela dans des dizaines d'endroits?
Dan
8

Cela n'a rien à voir avec le copier-coller. Si vous prenez du code d'ailleurs, à la seconde vous prenez le code, c'est votre code et votre responsabilité, donc qu'il soit copié ou écrit entièrement par vous-même ne fait aucune différence.

Dans vos alertes, vous prenez des décisions de conception. Des décisions de conception similaires devraient probablement être prises pour toutes les alertes. Il est donc probable que vous ayez une méthode quelque part "ShowAlertInAStyleSuitableForMyApplication" ou peut-être un peu plus courte, et cela devrait être appelé.

Vous aurez beaucoup de requêtes http avec une gestion des erreurs similaire. Vous ne devriez probablement pas dupliquer la gestion des erreurs encore et encore et encore, mais extraire la gestion des erreurs courantes. Surtout si votre gestion des erreurs est un peu plus élaborée (qu'en est-il des erreurs de temporisation, 401, etc.).

gnasher729
la source
6

La duplication est OK dans certaines circonstances. Mais pas dans celui-ci. Cette méthode est trop complexe. Il y a une limite inférieure, lorsque la duplication est plus facile que de «factoriser» une méthode.

Par exemple:

def add(a, b)
    return a + b
end

est stupide, faites juste a + b.

Mais lorsque vous devenez un peu, un tout petit peu plus complexe, vous êtes généralement bien au-dessus de la ligne.

foo.a + foo.b

devrait devenir

foo.total
def foo
    ...
    def total
        return self.a + self.b
    end
end

Dans votre cas, je vois quatre "méthodes". Probablement dans différentes classes. Un pour faire la demande, un pour obtenir la réponse, un pour afficher les erreurs et une sorte de rappel à appeler après le retour de la réponse pour traiter la réponse. Personnellement, j'ajouterais probablement une sorte de "wrapper" en plus pour faciliter les appels.

En fin de compte, pour faire une demande Web, je voudrais qu'un appel ressemble à quelque chose comme:

Web.Post(URI, Params, ResponseHandler);

Cette ligne est ce que j'aurais partout dans mon code. Ensuite, lorsque je devais apporter des modifications à «la façon dont j'obtiens des choses», je pouvais le faire rapidement, avec beaucoup moins d'efforts.

Cela maintient également le code SEC et aide à SRP .

coteyr
la source
0

Dans un projet de toute taille / complexité, je veux pouvoir trouver du code quand j'en ai besoin aux fins suivantes:

  1. Réparez-le quand il est cassé
  2. Changer la fonctionnalité
  3. Réutilisez-le.

Ne serait-il pas agréable de rejoindre un projet en cours ou de continuer à travailler sur un projet pendant plusieurs années et lorsqu'une nouvelle demande de "se connecter à Internet et d'afficher les résultats de la connexion" était dans un endroit facile à trouver en raison d'un une bonne conception au lieu de compter sur une recherche dans le code pour une requête http? C'est probablement plus facile à trouver avec Google de toute façon.

Ne vous inquiétez pas, je suis le nouveau mec et je vais refactoriser ce bloc de code parce que je suis tellement contrarié de rejoindre cette équipe désemparée avec la base de code horrible ou je suis maintenant sous beaucoup de pression comme le reste de vous et il suffit de le copier-coller. Au moins, cela éloignera le patron de mon dos. Ensuite, lorsque le projet sera véritablement identifié comme un désastre, je serai le premier à recommander de le réécrire en copiant et collant la version dans le cadre le plus récent et le plus efficace qu'aucun de nous ne comprend.

JeffO
la source
0

Une classe et / ou une fonction, c'est mieux, du moins à mon avis. Pour une fois, cela rend le fichier plus petit, ce qui représente un gain très important si vous traitez avec des applications Web ou des applications pour des appareils avec peu de stockage (IoT, téléphones plus anciens, etc.)

Et évidemment, le meilleur point est que si vous avez quelque chose à changer en raison de nouveaux protocoles, etc. trouver et changer.

J'ai écrit un interpréteur SQL entier pour pouvoir mieux passer de MySQL à MySQLi en PHP, car je dois juste changer mon interprète et tout fonctionne, bien que ce soit un exemple un peu extrême.

My1
la source
-1

Pour décider si un code doit être dupliqué ou déplacé vers une fonction appelée deux fois, essayez de déterminer laquelle est la plus probable:

  1. Il sera nécessaire de changer les deux utilisations du code de la même manière.

  2. Il sera nécessaire de modifier au moins une utilisation du code afin qu'elles diffèrent.

Dans le premier cas, il sera probablement préférable qu'une seule fonction gère les deux utilisations; dans ce dernier cas, il sera probablement préférable d'avoir un code séparé pour les deux usages.

Pour décider si un morceau de code qui sera utilisé une fois doit être écrit en ligne ou retiré dans une autre fonction, déterminez comment on décrirait complètement le comportement requis de la fonction. Si une description complète et précise du comportement requis de la fonction serait aussi longue ou plus longue que le code lui-même, le déplacement du code dans une fonction distincte peut rendre les choses plus difficiles à comprendre plutôt que plus faciles. Il peut toujours être utile de le faire s'il existe une forte probabilité qu'un deuxième appelant doive utiliser la même fonction, et toute modification future de la fonction devra affecter les deux appelants, mais en l'absence d'une telle considération, la lisibilité favoriserait le fractionnement des choses au niveau où le code et une description de son comportement requis seraient à peu près aussi longs.

supercat
la source