27

Ecco un semplice C++ frammento:avuto una risposta inaspettata dal x y: z espressione

int x1 = 10, x2=20, y1=132, y2=12, minx, miny, maxx, maxy; 
x1<=x2 ? minx=x1,maxx=x2 : minx=x2,maxx=x1; 
y1<=y2 ? miny=y1,maxy=y2 : miny=y2,maxy=y1; 
cout<<"minx="<<minx<<"\n"; 
cout<<"maxx="<<maxx<<"\n"; 
cout<<"miny="<<miny<<"\n"; 
cout<<"maxy="<<maxy<<"\n"; 

ho pensato che il risultato dovrebbe essere:

minx=10 
maxx=20 
miny=12 
maxy=132 

Ma in realtà il risultato è:

minx=10 
maxx=10 
miny=12 
maxy=132 

Qualcuno potrebbe dare una spiegazione del perché maxx non è 20? Grazie.

+7

Parentesi che risolve il problema ... –

+24

Ancora un altro motivo per "non cercare di essere intelligenti" con espressioni condizionali e usare invece "if". Il compilatore farà la stessa cosa in ogni caso [presumendo che tu aggiunga la parentesi pertinente in modo che faccia ciò che realmente volevi]. Le espressioni ternarie possono essere utili a volte, ma questo è un buon esempio di cosa NON fare con loro. –

+0

(E comunque, perché no: 'maxx = x1> x2? X1: x2'? –

risposta

40

causa di precedenza degli operatori, l'espressione viene analizzato in questo modo:

(x1<=x2 ? minx=x1,maxx=x2 : minx=x2), maxx=x1; 

è possibile risolvere questo con:

(x1<=x2) ? (minx=x1,maxx=x2) : (minx=x2, maxx=x1); 

E in realtà non è necessario le prime due coppie di parentesi. Also check this question.

25

La precedenza dell'operatore condizionale è maggiore di quella dell'operatore virgola, così

x1<=x2 ? minx=x1,maxx=x2 : minx=x2,maxx=x1; 

viene parentesi, come

(x1<=x2 ? minx=x1,maxx=x2 : minx=x2),maxx=x1; 

Così l'ultima assegnazione avviene indipendentemente dalla condizione.

Per risolvere il problema, è possibile

  • uso parentesi:

    x1 <= x2 ? (minx = x1, maxx = x2) : (minx = x2, maxx = x1); 
    

    (non lo fai necessità le parentesi nel ramo true, ma è IMO meglio avere anche loro).

  • uso due condizionali:

    minx = x1 <= x2 ? x1 : x2; 
    maxx = x1 <= x2 ? x2 : x1; 
    
  • uso un if:

    if (x1 <= x2) { 
        minx = x1; 
        maxx = x2; 
    } else { 
        minx = x2; 
        maxx = x1; 
    } 
    

compilato con o senza ottimizzazioni, la versione if e la parentesi, singolo condizionale con virgole producono lo stesso assemblando sia sotto gcc (4.7.2) che clang (3.2), è ragionevole aspettarselo anche da altri compilatori. La versione con i due condizionali produce assemblaggi diversi, ma con le ottimizzazioni, entrambi questi compilatori emettono solo una istruzione cmp per quello.

A mio avviso, la versione if è la più semplice per verificare la correttezza di, quindi preferibile.

+1

+1 per testare la differenza nel codice generato e fornire un argomento di leggibilità – fluffy

1

A causa della precedenza degli operatori:

(x1<=x2 ? minx=x1,maxx=x2 : minx=x2),maxx=x1

si può risolvere con:

int x1=10, x2=20, y1=132, y2=12, minx, miny, maxx, maxy; 
x1<=x2 ? (minx=x1,maxx=x2) : (minx=x2,maxx=x1); 
y1<=y2 ? (miny=y1,maxy=y2) : (miny=y2,maxy=y1); 
cout<<"minx="<<minx<<"\n"; 
cout<<"maxx="<<maxx<<"\n"; 
cout<<"miny="<<miny<<"\n"; 
cout<<"maxy="<<maxy<<"\n"; 
4

Domanda interessante sia per quanto riguarda le operazioni di precedenza e la generazione del codice.

OK, l'operazione , ha una priorità MOLTO bassa (minima, vedere reference table). A causa di questo fatto il codice è la stessa come le seguenti righe:

((x1<=x2) ? minx=x1,maxx=x2 : minx=x2),maxx=x1; 
((y1<=y2) ? miny=y1,maxy=y2 : miny=y2),maxy=y1; 

In realtà solo C/C++ grammatica impedisce primo , dallo stesso comportamento.

Ancora un altro posto molto pericoloso in precedenza di operazioni C/C++ è operazioni bituminose e confronto. Considerare circa il seguente frammento:

int a = 2; 
int b = (a == 2|1); // Looks like check for expression result? Nope, results 1! 

Guardando al futuro, mi consiglia di riscrivere il frammento in questo modo mantenere l'equilibrio tra efficienza e leggibilità:

minx = ((x1 <= x2) ? x1 : x2); 
maxx = ((x1 <= x2) ? x2 : x1); 
miny = ((y1 <= y2) ? y1 : y2); 
maxy = ((y1 <= y2) ? y2 : y1); 

fatto più interessante di questo frammento di codice è tale lo stile potrebbe risultare il codice più efficace per alcune architetture come ARM a causa di flag di bit di condizione nel set di istruzioni della CPU (la duplicazione della condizione non costa nulla e di più, il compilatore di punti ai frammenti del codice di destra).

5

mentre altri hanno spiegato quale sia la causa del problema è, credo che la soluzione "migliore" dovrebbe essere quello di scrivere il condizionale con se:

int x1 = 10, x2=20, y1=132, y2=12, minx, miny, maxx, maxy; 
if (x1<=x2) 
{ 
    minx=x1; 
    maxx=x2; 
} 
else 
{ 
    minx=x2; 
    maxx=x1; 
} 
if (y1<=y2) 
{ 
    miny=y1; 
    maxy=y2; 
} 
else 
{ 
    miny=y2; 
    maxy=y1; 
} 

Sì, è più linee più a lungo, ma è anche più facile per leggere e chiarire esattamente cosa succede (e se hai bisogno di attraversarlo nel debugger, puoi facilmente vedere in che direzione va).

Qualsiasi compilatore moderno dovrebbe essere in grado di convertire uno di questi in assegnamenti condizionali piuttosto efficienti che faccia un buon lavoro di evitare rami (e quindi "previsione di ramo errato").

ho preparato un piccolo test, che ho compilato utilizzando

g++ -O2 -fno-inline -S -Wall ifs.cpp 

Ecco la fonte (ho dovuto farlo parametri per garantire il compilatore non solo calcolare direttamente il valore corretto e solo fare mov $12,%rdx, ma in realtà ha un confrontare e decidere con è più grande):

void mine(int x1, int x2, int y1, int y2) 
{ 
    int minx, miny, maxx, maxy; 
    if (x1<=x2) 
    { 
    minx=x1; 
    maxx=x2; 
    } 
    else 
    { 
    minx=x2; 
    maxx=x1; 
    } 
    if (y1<=y2) 
    { 
    miny=y1; 
    maxy=y2; 
    } 
    else 
    { 
    miny=y2; 
    maxy=y1; 
    } 

    cout<<"minx="<<minx<<"\n"; 
    cout<<"maxx="<<maxx<<"\n"; 
    cout<<"miny="<<miny<<"\n"; 
    cout<<"maxy="<<maxy<<"\n"; 
} 

void original(int x1, int x2, int y1, int y2) 
{ 
    int minx, miny, maxx, maxy; 
    x1<=x2 ? (minx=x1,maxx=x2) : (minx=x2,maxx=x1); 
    y1<=y2 ? (miny=y1,maxy=y2) : (miny=y2,maxy=y1); 
    cout<<"minx="<<minx<<"\n"; 
    cout<<"maxx="<<maxx<<"\n"; 
    cout<<"miny="<<miny<<"\n"; 
    cout<<"maxy="<<maxy<<"\n"; 
} 

void romano(int x1, int x2, int y1, int y2) 
{ 
    int minx, miny, maxx, maxy; 

    minx = ((x1 <= x2) ? x1 : x2); 
    maxx = ((x1 <= x2) ? x2 : x1); 
    miny = ((y1 <= y2) ? y1 : y2); 
    maxy = ((y1 <= y2) ? y2 : y1); 
    cout<<"minx="<<minx<<"\n"; 
    cout<<"maxx="<<maxx<<"\n"; 
    cout<<"miny="<<miny<<"\n"; 
    cout<<"maxy="<<maxy<<"\n"; 
} 

int main() 
{ 
    int x1=10, x2=20, y1=132, y2=12; 
    mine(x1, x2, y1, y2); 
    original(x1, x2, y1, y2); 
    romano(x1, x2, y1, y2); 
    return 0; 
} 

Il codice generato è simile al seguente:

_Z4mineiiii: 
.LFB966: 
    .cfi_startproc 
    movq %rbx, -32(%rsp) 
    movq %rbp, -24(%rsp) 
    movl %ecx, %ebx 
    movq %r12, -16(%rsp) 
    movq %r13, -8(%rsp) 
    movl %esi, %r12d 
    subq $40, %rsp 
    movl %edi, %r13d 
    cmpl %esi, %edi 
    movl %edx, %ebp 
    cmovg %edi, %r12d 
    cmovg %esi, %r13d 
    movl $_ZSt4cout, %edi 
    cmpl %ecx, %edx 
    movl $.LC0, %esi 
    cmovg %edx, %ebx 
    cmovg %ecx, %ebp 
     .... removed actual printout code that is quite long and unwieldy... 
_Z8originaliiii: 
    movq %rbx, -32(%rsp) 
    movq %rbp, -24(%rsp) 
    movl %ecx, %ebx 
    movq %r12, -16(%rsp) 
    movq %r13, -8(%rsp) 
    movl %esi, %r12d 
    subq $40, %rsp 
    movl %edi, %r13d 
    cmpl %esi, %edi 
    movl %edx, %ebp 
    cmovg %edi, %r12d 
    cmovg %esi, %r13d 
movl $_ZSt4cout, %edi 
cmpl %ecx, %edx 
movl $.LC0, %esi 
cmovg %edx, %ebx 
cmovg %ecx, %ebp 
     ... print code goes here ... 
_Z6romanoiiii: 
    movq %rbx, -32(%rsp) 
    movq %rbp, -24(%rsp) 
    movl %edx, %ebx 
    movq %r12, -16(%rsp) 
    movq %r13, -8(%rsp) 
    movl %edi, %r12d 
    subq $40, %rsp 
    movl %esi, %r13d 
    cmpl %esi, %edi 
    movl %ecx, %ebp 
    cmovle %edi, %r13d 
    cmovle %esi, %r12d 
movl $_ZSt4cout, %edi 
cmpl %ecx, %edx 
movl $.LC0, %esi 
cmovle %edx, %ebp 
cmovle %ecx, %ebx 
     ... printout code here.... 

Come potete vedere, mine e original sono identici e romano utilizza registri leggermente diversi e una diversa forma di cmov, ma altrimenti fanno la stessa cosa con lo stesso numero di istruzioni.

+1

+1 Il codice inserito nella domanda è esattamente il tipo di codice che rende molte organizzazioni semplicemente vietare l'operatore ternario. L'operatore ternario ha il suo posto, ma l'uso registrato non è quel posto. –

+1

Re * Sì, esso Le righe sono più lunghe * - Non sarebbe stato così male se avessi usato 1TBS. :) –

+0

@DavidHammen Sì, lo puoi modellare in un altro modo [questo è proprio come di solito scrivo il mio codice], ma probabilmente non (senza renderlo abbastanza disordinato) ridurlo a 2 righe ... Anche quattro ragionevolmente leggibili le linee lo stanno spingendo. Quindi l'affermazione è ancora "diverse righe più lunghe". E il mio punto era renderlo più leggibile, non adatto a una voce IOCCC. –

1

In C++ 11 è possibile utilizzare std::tie e std::make_pair per fare questo ovviamente-corretta-at-a-glance (TM)

#include <tuple> 
#include <utility> 
#include <iostream> 

using namespace std; 

int main() 
{ 
    int x1 = 10, x2=20, y1=132, y2=12, minx, miny, maxx, maxy; 

    tie(minx, maxx) = (x1 <= x2)? make_pair(x1, x2) : make_pair(x2, x1); 
    tie(miny, maxy) = (y1 <= y2)? make_pair(y1, y2) : make_pair(y2, y1); 

    cout<<"minx="<<minx<<"\n"; 
    cout<<"maxx="<<maxx<<"\n"; 
    cout<<"miny="<<miny<<"\n"; 
    cout<<"maxy="<<maxy<<"\n"; 
} 

online output.

Questo è semanticamente equivalente a tutte le altre soluzioni pubblicate e con qualsiasi compilatore di ottimizzazione decente, non ha nessun tipo di sovraccarico. È sintatticamente molto meglio perché ha

  • un minimo di ripetizione codice,
  • 4 assigned-to-variabili sono tutti sul lato sinistro della cessione, e
  • 4 assegnato-from le variabili sono tutte sulla destra.

Come una leggera variazione che generalizza a trovare puntatori al min e max elemento di sequenze, è possibile utilizzare std::minmax_element e il fatto che gli array prime hanno funzioni terzi begin() e end() (entrambe le caratteristiche C++ 11)

#include <algorithm> 
#include <tuple> 
#include <iostream> 

using namespace std; 

int main() 
{ 
    int x[] = { 10, 20 }, y[] = { 132, 12 }, *minx, *miny, *maxx, *maxy; 

    tie(minx, maxx) = minmax_element(begin(x), end(x)); 
    tie(miny, maxy) = minmax_element(begin(y), end(y)); 

    cout<<"minx="<<*minx<<"\n"; 
    cout<<"maxx="<<*maxx<<"\n"; 
    cout<<"miny="<<*miny<<"\n"; 
    cout<<"maxy="<<*maxy<<"\n"; 
} 

Online output.