2010-09-02 8 views
0

Je viens d'avoir une discussion avec un collègue où nous étions en désaccord sur lequel des extraits suivants était plus simple:Est-ce plus simple de grouper des tests ou de les séparer?

public boolean foo(int x, int y) { 
    if (x < 0) 
     return false; 
    if (y < 0) 
     return false; 
    // more stuff below 
} 

OU

public boolean foo(int x, int y) { 
    if (x < 0 || y < 0) 
     return false; 
    // more stuff below 
} 

Il est évident qui est plus courte; il est également évident que leur cyclomatic complexity est identique (donc pour cette définition de "simple", bien sûr, ils sont les mêmes).

Comment vous sentez-vous et pourquoi? Lequel est le plus lisible; qui est plus facile à déboguer?

Répondre

0

Je vais pour le second, avec un seul changement:

public boolean foo(int x, int y) { 
    if ((x < 0) || (y < 0)) 
     return false; 
    // more stuff below 
} 

La raison étant qu'il suit la façon dont un humain pourrait dire: dans un langage naturel (nous dirions « si l'un des les deux sont négatifs, alors la réponse est "non" "plutôt que" si le premier est négatif, la réponse est "non", si le second est négatif, la réponse est aussi "non" ").

0

Je pense que les styles suggèrent des choses différentes:

if (x < 0) 
    return false; 
if (y < 0) 
    return false; 

dit que quelque chose est fait si x < 0 et une autre chose se fait si y < 0.

Alors que

if (x < 0 || y < 0) 
    return false; 

implique qu'une chose est fait si l'une des conditions est vraie.

Cela dépend donc du code réel. Évidemment, vous ne diviseriez pas l'instruction si les deux expressions devaient faire la même chose car cela conduirait à dupliquer le code et se sentirait juste erroné.

4

Dans le cas complètement générique que vous décrivez, je choisirais le second parce que je déteste répéter le code.

Mais dans la vraie vie, je prendrais cette décision en fonction de si les deux tests sont liés.

Par exemple

if (user.isDisabled() || user.isSuspended()) 
    return false; 

les deux tests sont de savoir si l'utilisateur peut faire quelque chose.

mais

if (user.isDisabled()) 
    return false; 
if (catalog.isClosedForOrders()) 
    return false; 

un test est sur l'utilisateur, l'un est sur le système; Je séparerais ceux-ci pour la facilité de la maintenance quand on change plus tard indépendamment de l'autre.

1

Eh bien, en grande partie cela dépendrait de mon humeur à l'époque.

Je pense que la considération la plus objective serait à quel point x & y est étroitement lié. Vérification d'une variable pour une plage basse & - un si. Vérifiez une chaîne pour null et un int pour range - deux ifs.

0

noms Mieux, il est vraiment clair ...

public boolean foo(int x, int y) { 
    var hasInvalidArgument = (x < 0 || y < 0); 

    if (hasInvalidArgument) 
     return false; 

    // more stuff below 
} 

Vous n'avez pas besoin de regarder l'expression ... le nom de la variable vous dire ce que le test est