Vraag if (x! = y) vs if (x == y)


Ik heb het PMD-plug-in in Eclipse tegen mijn code en ik krijg een waarschuwing met hoge prioriteit voor code die lijkt op de onderstaande code:

 if(singleRequest !=null){
   // do my work
 }else{
   // do my other work
 }

PMD zegt `Avoid if (x != y) ..; else ..;

En de beschrijving van de fout ziet er als volgt uit:

In an "if" expression with an "else" clause, avoid negation in
the test.  For example, rephrase:
if (x != y) diff(); else same();
as:
if (x == y) same(); else diff();
Most "if (x != y)" cases without an "else" are often return

maar ik begrijp de impact op mijn code nog steeds niet. Als iemand me een voorbeeld zou kunnen geven, zou ik het op prijs stellen.


22
2017-11-06 18:42


oorsprong


antwoorden:


Een aantal PMD-regels zijn meer stijladviezen dan correctheidswaarschuwingen. Als u het niet eens bent met deze regel of als de regel niet overeenkomt met de coderingsstandaarden van uw project, kunt u overwegen waarschuwingen onderdrukken of zelfs configureren van PMD om alleen de regels af te dwingen die u bevalt


31
2017-11-06 19:07



PMD is een hulpmiddel. PMD werkt op basis van heuristieken. Iemand heeft deze heuristiek besloten; dat negatief conditionals met andere uitspraken is niet "goede stijl".

In dit geval, zoals ik in mijn opmerkingen heb betoogd, de geposte code is hoe ik zou het schrijven. (In het bijzonder met x != null, maar niet uitsluitend voor dit construct.)

Dit komt omdat ik niet naar de kijker kijk voorwaardelijk (uitgezonderd omdat het vereenvoudigd kan worden, bijvoorbeeld het verwijderen van dubbele negatieven zoals getoond door Jim Kin) maar ik kijk eerder naar de logica van de takken of "stroom".

Dat wil zeggen, ik plaats het positieve tak eerste. In dit geval betwist ik dat

if (x != null) {
  doValid         // positive branch
} else {
  doFallback
}

is semantisch equivalent aan

if (isValid(x)) { // it looks like a "positive conditional" now
  doValid         // still positive branch
} else {
  doFallback
}

en is dus positief tak eerste.

Natuurlijk hebben niet alle situaties zo'n "duidelijke" positieve stroom, en sommige uitdrukkingen kunnen op een negatieve manier veel gemakkelijker tot uitdrukking worden gebracht. In deze gevallen zal ik de takken "omkeren" - vergelijkbaar met wat PMD suggereert - meestal met een opmerking waarin de actie aan de bovenkant van het blok wordt vermeld als de positieve tak/ flow was omgekeerd.

Een andere factor die van invloed kan zijn op de voorwaardelijke keuze die wordt gebruikt, is "direct scope-exit" -takken zoals:

if (x == null) {
  // return, break, or
  throw new Exception("oops!");
} else {
  // But in this case, the else is silly
  // and should be removed for clarity (IMOHO) which,
  // if done, avoids the PMD warning entirely
} 

Dit is hoe ik  consistent (een paar uitzonderingen daargelaten) mijn code: if (x != null) { .. }. Gebruik de beschikbare tools; en laat ze werken voor u. Zie Steven's antwoord voor hoe PMD hier kan worden geconfigureerd voor een meer geschikte "smaak".


13
2017-11-06 18:57



Het is een kwestie van leesbaarheid. Overwegen

if ( x != y ) 
{
}
else  // "if x doesn't not equal y"
{
}

vs.

if ( x == y )
{
}
else  // "if x doesn't equal y"
{
}

Het laatste voorbeeld is meer direct herkenbaar. Let wel, ik zie niets verkeerds met het gebruik van negatieven ... het kan veel logischer zijn, overweeg

if ( x != null )...

8
2017-11-06 18:44



De enige reden dat ik zou vermijden om de negatieve casus te gebruiken, is of het dubbele-negatieven opleverde, wat verwarrend kan zijn.

bijv.

if (!checkbox.disabled) {
    // checkbox is enabled
} 
else {
    // checkbox is disabled
}

4
2017-11-06 18:49



Wie leest uw code? Je doet. De compiler doet dat. Of misschien de assistent van de docent. Een collega, die geen verschil kan maken tussen == en! =? Hoop het niet.

Ik kan alleen maar denken dat negatieven slecht zijn in complexe uitdrukkingen. (Context zijnde: tenminste voor mij. Ik weet dat ik gefrustreerd ben in het debuggen in mijn hoofd while(!expr && !expr2 || expr3) { })

ch=getch(); if (ch!='a')  is een patroon dat gemakkelijk wordt uitgebreid naar
if (ch!='a' || ch!='b') wat altijd waar is, terwijl het semantisch correct klinkt.

Uit het oogpunt van prestaties, is het het beste om de kansen te sorteren.

if (more_probable) {
      ....
      unconditional_jump_to_end_of_block;
} else {
   ...
}

Deze keuze moet leiden tot betere prestaties, omdat er geen verkeerde voorspellingstraf is in de meer waarschijnlijke vertakking.

if (p && p->next) geëvalueerd vanuit prestatie-oogpunt geeft slechte resultaten.


3
2017-11-06 19:00



U moet voorkomen dat er "niet gelijk aan" is in de if-toestand. Dit komt omdat wanneer iemand anders naar je code kijkt, er een reële mogelijkheid is dat de persoon de! = Negeert en mogelijk een verkeerde conclusie trekt over de logica van je programma.

Voor uw geval moet u wellicht de if-logica uitwisselen met else logic and change! = To ==


1
2017-11-06 18:46



Het is een evenwicht tussen code-leesbaarheid en code-indeling. De waarschuwing suggereert in feite dat het verwarrend is voor mensen die de code lezen om door de negatie van een negatief te navigeren.

Mijn persoonlijke vuistregel is dat, wat je ook verwacht dat het "normale" geval is, datgene waarop je moet testen in de if. Overwegen:

 if (x != y) {
   // do work here...
 } else {
   throw new IllegalArgumentException();
 }

In deze situatie zou ik zeggen dat het belangrijke werk wordt gedaan in de x != y geval, dus daar moet je voor testen. Dit komt omdat ik graag de code orden, zodat het belangrijke werk voorop staat, gevolgd door de behandeling voor uitzonderlijke gevallen.


1
2017-11-06 18:47



Het is omdat "goede stijl" zegt dat als mogelijke tests "positief" zouden moeten zijn, dus:

if (singleRequest == null){
   // do my other work
} else {
   // do my work
}

Is gemakkelijker te lezen omdat de test "positief" is (dwz "is gelijk aan" en niet "niet gelijk aan"), en uiteindelijk leidt een betere leesbaarheid tot minder fouten.

bewerkt

Dit is met name het geval bij tests zoals:

if (!str.equals("foo")) {

je kunt gemakkelijk de missen ! aan de voorkant, maar als je de test positief maakt, is het een stuk schoner.

De enige keer dat u een negatieve test zou moeten doen, is wanneer er nee is else blok - dan is een negatieve test onvermijdelijk, tenzij je een lege hebt true blok, wat op zichzelf als een stijlprobleem wordt beschouwd.


0
2017-11-06 18:47