È un anti-pattern per usare il tipo di ritorno di un compito come parte di una condizione?

Tecnicamente è OK

Non c'è niente di sbagliato nell'usare il simbolo = sia per l'assegnazione che come operatore (che restituisce il valore dell'assegnazione), ed è certamente legale e conciso. La domanda è se è chiaro e ciò dipenderà da chi lo sta leggendo.

Perché è evitato

Nella mia esperienza, la maggior parte degli sviluppatori evita il singolo = in un'istruzione if perché è un modo facile per commettere un errore e l'errore è spesso difficile da rilevare e ha un effetto molto negativo. Infatti Visual Studio ti avviserà solo perché è un problema molto comune. Penso che sia un segno abbastanza strong che dovresti evitarlo.

Questo è ancora peggio

Ciò che è veramente grave è quando l'operatore viene utilizzato più avanti in un'espressione in cortocircuito, come questo:

if (condition1 | model.MyBoolean = condition2) {}

Quanto sopra funzionerà, ma non lo farà:

if (condition1 || model.MyBoolean = condition2) {}

Se condition1 è falso, l'assegnazione non avverrà !!! Ciò potrebbe non essere ovvio per chi sta modificando il codice e può facilmente introdurre un errore logico catastrofico.

Cosa potrebbe essere OK

D'altra parte, l'utilizzo di% singolo= come questo non è raro in certi altri costrutti, come ad esempio in un ciclo while:

while (bytesRead = streamReader.Read(buffer, x, y) > 0)
{
    //Do something with the buffer
}

Il ciclo precedente leggerà byte da un flusso e registrerà la lunghezza dei byte letti, e uscirà dal ciclo se bytesRead è zero.

Ho visto questo costrutto dappertutto. Perché mi è familiare, è abbastanza chiaro per me, e finora non mi ha causato alcuna confusione quando l'ho incontrato.

Linea inferiore

Devi decidere autonomamente se il tuo codice è chiaro. Secondo me (e Microsoft), non lo è, e non lo farei, ma forse la tua squadra è diversa. Poi di nuovo probabilmente è meglio sbagliare con cautela.

Ho capito che vuoi essere conciso. Penso che la maggior parte dei poster su questo forum pensi che la concisione sia molto, molto meno importante della chiarezza. Inoltre, ecco un post sul blog sulla stesura di "Really Obvious Code" e perché è una buona idea. Il tuo codice non è davvero ovvio.

Mentre il linguaggio lo consente, non è qualcosa che (almeno nella mia esperienza) si usa molto. Dubito che troppe persone si lascerebbero scagliare da esso, ma faranno in modo che debbano fermarsi e leggere più attentamente. E secondo me, dovrei essere in grado di leggere il codice senza dover spendere così tanto sforzo mentale su cose del genere.

Inoltre, le dichiarazioni if non sono un luogo abituale per i compiti, quindi le persone non cercheranno che ciò accada lì. E per l'appassionato osservatore, potrebbero pensare che sia stato un errore e cercare di "correggere" quello che probabilmente è un bug.

Vorrei andare per qualcosa di simile:

model.MyBoolean = [condition];
if (model.MyBoolean) { ... }

Perché è molto facile interpretare erroneamente = per == , specialmente se ti aspetti quest'ultimo. È il tipo di cosa che ho letto, poi qualche tempo dopo mi rendo conto che qualcosa non va, torna al if e vai "Oh, giusto, è lì che è stato impostato".

Inoltre, se hai mai avuto bisogno di modificare questa condizione in seguito a qualcosa di leggermente più complesso, è una grande fonte di bug - l'assegnazione in condizioni non funziona molto bene con gli operatori di cortocircuito!

Sembra che tu stia testando il risultato dell'assegnazione di un valore a model.MyBoolean . Lo trovo un po 'strano da leggere. Io preferisco leggere codice come

model.MyBoolean = ([Some Condition]) ;
if (model.MyBoolean)
{
    /* do stuff */
}