Question

Lequel de ces est mieux pour maintenabilité?

if (byteArrayVariable != null)
    if (byteArrayVariable .Length != 0)
         //Do something with byteArrayVariable 

ou

if ((byteArrayVariable != null) && (byteArrayVariable.Length != 0))
  //Do something with byteArrayVariable 

Je préfère lire et écrire le deuxième, mais je me souviens avoir lu dans le code complet que faire les choses comme ça est mauvais pour la maintenabilité.

Ceci est parce que vous comptez sur la langue pour ne pas évaluer la deuxième partie du if si la première partie est fausse et non toutes les langues faire. (La seconde partie va lever une exception si évaluée avec un byteArrayVariable null).

Je ne sais pas si cela est vraiment quelque chose à se soucier ou non, et je voudrais des commentaires généraux sur la question.

Merci.

Était-ce utile?

La solution

Je pense que la deuxième forme est très bien, et aussi représente plus clairement ce que vous essayez de faire. Vous dites que ...

Je me souviens avoir lu dans le code complet que faire les choses comme ça est mauvais pour la maintenabilité. Ceci est parce que vous comptez sur la langue pour ne pas évaluer la deuxième partie du cas si la première partie est fausse et non toutes les langues faire.

Peu importe si toutes les langues faire. Vous écrivez dans un langue particulière, il importe que si que langue fait cela. Dans le cas contraire, vous dites essentiellement que vous ne devriez pas utiliser les fonctions d'une langue particulière parce que d'autres langues pourraient ne pas soutenir ces fonctionnalités, et c'est tout simplement ridicule.

Autres conseils

Je ne suis personne surprise en a parlé (si tout va bien je ne suis pas une mauvaise interprétation de la question) - mais les deux types de Suck

La meilleure alternative serait d'utiliser du début des sorties (en plaçant une déclaration de retour au début du code si aucun autre code doit exécuter). Cela rend l'hypothèse que votre code est relativement bien refactorisé, et que chaque méthode a un but de concision.

A ce moment-là que vous feriez quelque chose comme:

if ((byteArrayVariable == null) || (byteArrayVariable.Length != 0)) {
  return; // nothing to be done, leaving
}

// Conditions met, do something

Il peut sembler un peu comme la validation - et c'est précisément ce qu'il est. Il confirme que les conditions sont remplies pour que la méthode faire des choses de ce -. Les deux options que vous propose hid la logique réelle dans les contrôles pré-condition

Si votre code est beaucoup de spaghettis (méthodes Loooong, beaucoup de ifs imbriquées avec la logique dispersée entre eux, etc.), alors vous devriez vous concentrer sur le plus gros problème d'obtenir votre code en forme avant que les petits problèmes stylistiques. En fonction de votre environnement, et la gravité du désordre, il y a quelques grands outils d'aide (par exemple Visual Studio a un « extrait Méthode » Fonction refactoring).


Comme Jim mentionne, à propos de il y a encore place pour le débat comment à structurer la sortie précoce. L'alternative à ce qui est ci-dessus serait:

if (byteArrayVariable == null) 
  return; // nothing to be done, leaving

if (byteArrayVariable.Length != 0)
  return;

// Conditions met, do something

Ne perdez pas de temps à essayer de piéger pour chaque non ce condition. Si vous pouvez disqualifier les données ou la condition, puis le faire le plus rapidement possible.

if (byteArrayVariable == null) Exit;
if (byteArrayVariable.Length == 0) Exit;
// do something

Cela vous permet d'adapter votre code beaucoup plus facile pour les nouvelles conditions

if (byteArrayVariable == null) Exit;
if (byteArrayVariable.Length == 0) Exit;
if (NewCondition == false) Exit;
if (NewerCondition == true) Exit;
// do something

Votre exemple est l'exemple parfait de la raison pour laquelle ifs imbriquées sont une mauvaise approche pour la plupart des langues qui prennent en charge correctement la comparaison booléenne. Imbriquant les ifs crée une situation où votre intention est pas du tout clair. Est-il nécessaire que le second if suit immédiatement la première? Ou est-ce simplement parce que vous ne l'avez pas mis une autre opération là encore?

if ((byteArrayVariable != null) && (byteArrayVariable.Length != 0))
  //Do something with byteArrayVariable 

Dans ce cas, ces jolis beaucoup doit être ensemble. Ils agissent en tant que garde pour vous assurer de ne pas effectuer une opération qui entraînerait une exception. En utilisant ifs imbriquée, vous le laissez à l'interprétation de la personne que vous suivant pour savoir si les deux représentent un garde en deux parties ou d'une autre logique de branchement. En les combinant en un seul if je déclare que. Il est beaucoup plus difficile de se faufiler à l'intérieur d'une opération de là où il ne devrait pas être.

Je privilégiera toujours une communication claire de mon intention sur l'affirmation stupide de quelqu'un qu'il « ne fonctionne pas dans toutes les langues ». Devinez quoi ... throw ne fonctionne pas dans toutes les langues non plus, que cela veut dire que je ne devrais pas l'utiliser lors du codage en C # ou Java et devrait compter plutôt sur des valeurs de retour pour signaler quand il y avait un problème?

Cela dit, il est beaucoup plus facile à lire pour l'inverser et juste revenir sur la méthode si l'est pas satisfait que STW dit dans leur réponse.

Dans ce cas, vos deux clauses sont directement liés, de sorte que la 2ème forme est préférable.

Si elles ne sont pas liés (par exemple une série de clauses de garde sur les conditions différentes) alors je préfère la 1ère forme (ou je Refactor une méthode avec un nom qui représente ce que la condition de composé représente en fait).

Si vous travaillez dans Delphi, la première est, en général, plus sûr. (Pour l'exemple donné, il n'y a pas grand-chose à gagner à l'aide ifs imbriquées.)

Delphi vous permet de spécifier si oui ou non des expressions booléennes évaluer ou « court-circuit », par des commutateurs du compilateur. Faire façon # 1 (ifs imbriqués) vous permet de vous assurer que la deuxième clause exécute si et seulement si (et strictement après ) la première clause est évaluée à vrai.

Le second style peut se retourner contre VBA car si le premier test est si l'objet généré, il sera toujours faire le second test et sortie d'erreur. Je viens de l'habitude en VBA lorsqu'ils traitent avec la vérification des objets à utiliser toujours la première méthode.

La deuxième forme est plus lisible, surtout si vous utilisez {} blocs autour de chaque article, parce que la première forme conduira à blocs imbriqués {} et plusieurs niveaux d'indentation, qui peut être une douleur à lire (vous devez compter les espaces d'indentation pour déterminer où chaque bloc se termine).

Je les trouve à la fois lisible, et je n'accepte pas l'argument selon lequel vous devriez vous inquiéter au sujet du code maintenabilité si vous étiez aux langues de commutation.

Dans certaines langues la deuxième option fonctionne mieux donc il serait le choix évident alors.

Je suis avec toi; Je fais la deuxième façon. Pour moi, il est logiquement plus claire. La préoccupation que certaines langues exécuteront la deuxième partie, même si la première est évaluée à false semble un peu ridicule pour moi, comme je l'ai jamais dans ma vie écrit un programme qui RAN dans « certaines langues »; mon code semble toujours exister dans une langue spécifique, qui soit capable de gérer que la construction ou ne peut pas. (Et si je ne suis pas sûr que la langue spécifique peut gérer, que quelque chose que je vraiment besoin d'apprendre, ne l'est pas.)

Dans mon esprit, le danger avec la première façon de le faire est qu'il me laisse vulnérable à mettre accidentellement dans le code en dehors de ce bloc imbriqué if quand je ne voulais pas. Ce qui, certes, est à peine une énorme préoccupation, mais il semble plus raisonnable que fretting pour savoir si mon code est agnostique de la langue.

Je préfère la deuxième option, car il est plus facile à lire.

Si la logique vous la mise en œuvre est plus complexe (vérifier qu'une chaîne est non nulle et non vide est juste un exemple), vous pouvez faire un refactoring utile: extraire une fonction booléenne. Je suis avec @mipadi sur la remarque « Code Complete » ( « il n'a pas d'importance si toutes les langues font que ... ») Si le lecteur de votre code ne souhaite pas regarder à l'intérieur de la fonction extraite, l'ordre dans lequel opérandes booléens sont évaluées devient une question sans intérêt pour eux.

if ((byteArrayVariable != null)
 && (byteArrayVariable.Length != 0)) {
    //Do something with byteArrayVariable 

mais qui est essentiellement la deuxième option.

Licencié sous: CC-BY-SA avec attribution
scroll top