Su Twitter, qualche settimana fa, è comparsa quest’immagine:
Si tratta di C#, ma possiamo facilmente riscriverla in C++ (e lo faremo, tra poco).
Nelle risposte a questo tweet, molti deridevano il codice, o si lamentavano dell’uso di “numeri magici”, mentre altri ancora discutevano dell’eccessiva lunghezza (implicando un costo di mantenimento proporzionale al numero di linee). Ero tentato di fare lo stesso, ma qualcosa mi ha fermato.
Mi sono reso conto che nessuno stava chiedendosi “Qual è il problema originale?”.
Assumiamo, per un attimo, che il problema fosse esattamente quello risolto, e cioè colorare precisamente 10 palle, con un numero di palle proporzionale alla percentuale (o alla frazione). Per questo problema, la soluzione presentata sarebbe davvero così terribile?
Ho deciso quindi di fare un esercizio di stile, buttando alle ortiche un intera serata su una barra di progresso fatta con 10 palle.
Il codice
Trattandosi di un blog di C++, il primo step sarà quello di riscrivere il codice nel nostro linguaggio d’elezione. L’ho migliorato solo un po’, rimuovendo metà delle costanti… ma spero siate d’accordo che, nello spirito, non è troppo distante dall’originale.
const char * ProgressBugFixedSimplified(double percent)
{
if (percent == 0.0) return "..........";
if (percent <= 0.1) return "o.........";
if (percent <= 0.2) return "oo........";
if (percent <= 0.3) return "ooo.......";
if (percent <= 0.4) return "oooo......";
if (percent <= 0.5) return "ooooo.....";
if (percent <= 0.6) return "oooooo....";
if (percent <= 0.7) return "ooooooo...";
if (percent <= 0.8) return "oooooooo..";
if (percent <= 0.9) return "ooooooooo.";
return "oooooooooo";
}
Risolve il problema? Quasi.
Risoluzione del bug
Il mio unico contributo alla discussione su Twitter è stato semplicemente la correzione di quello che considero essere un bug.
Nel caso di percentuali negative, il codice qui sopra produce una barra piena, ma secondo me la barra dovrebbe essere vuota. In alcuni casi, infatti, approssimazioni numeriche possono produrre risultati leggermente negativi quando i numeri si avvicinano allo zero, e quindi rischieremmo di vedere una barra piena all’inizio del processo. È una soluzione semplice, dobbiamo solo cambiare il primo test e farlo diventare un <=. Detto fatto:
const char * ProgressBugFixedSimplified(double percent)
{
if (percent <= 0.0) return "..........";
if (percent <= 0.1) return "o.........";
if (percent <= 0.2) return "oo........";
if (percent <= 0.3) return "ooo.......";
if (percent <= 0.4) return "oooo......";
if (percent <= 0.5) return "ooooo.....";
if (percent <= 0.6) return "oooooo....";
if (percent <= 0.7) return "ooooooo...";
if (percent <= 0.8) return "oooooooo..";
if (percent <= 0.9) return "ooooooooo.";
return "oooooooooo";
}
Per analizzare i miei progressi, ho deciso delle metriche (arbitrarie) che rispecchiassero le maggiori critiche trovate nella discussione: la dimensione del codice, dei dati, il numero di chiamate a funzioni esterne, e la presenza di eccezioni. In tutti i test ho utilizzato GCC 12.2, con le impostazioni -O1 -std=c++20.
Quindi, per questa versione i risultati sono:
Codice: 206 byte
Dati: 121 byte
Chiamate: nessuna
Eccezioni: no
Esiste una soluzione più concisa, migliore o più furba per risolvere il problema? Ma certo, siamo informatici esperti, mostriamo tutta la nostra conoscenza in materia, e ingegneriziamo per bene una soluzione a questo problema!
Il ciclo
Ok, dobbiamo raccogliere abbastanza palle per creare la nostra stringa, e riempire il resto con puntini. Per semplicità useremo una std::string, perché non dovremmo?
std::string ProgressLoop(double percent)
{
const int elements = 10;
double threshold = percent * elements;
std::string result = "";
for (int i = 0; i < elements; ++i)
{
result += (i < threshold) ? "o" : ".";
}
return result;
}
Meglio?
Posso cambiare il numero di palle volendo, certo… ma questo non faceva parte delle richieste originali, giusto?
Il codice inizia ad essere meno leggibile, ma possiamo comunque capire cosa sta succedendo.
Codice: 425 byte (senza le dipendenze esterne)
Dati: 0 bytes
Chiamate: 8 – throw (3x), delete (2x), new (1x), unwind (1x), memcpy (1x)
Eccezioni: Si
Che disdetta! Il codice è di più, ed inoltre alloca, dealloca e copia memoria, e può persino lanciare delle eccezioni in tre punti differenti.
Ad essere onesti, probabilmente non avremo nessuna di queste chiamate, perché nel nostro caso siamo ben entro i limiti della SSO – Small String Optimization (per riferimento, vi rimando al post di Joel Laity libc++’s implementation of std::string, in inglese, che spiega l’argomento in modo molto chiaro). Ma comunque, il codice per gestire tutti quei potenziali problemi è lì.
Passare la memoria in giro
Possiamo evitare std::string e ritornare un array di caratteri? No, è vietatissimo.
Potremmo ritornare un puntatore a char? Certo, ma qualcuno dovrebbe allocarlo, oppure ricevere il puntatore tramite un parametro: l’interfaccia sarebbe più complessa, e potenzialmente causerebbe più errori e confusione.
Ma tagliamola corta, e andiamo dritti a vedere
il vincitore!
Eccolo qui, il mio miglior codice che non alloca, ritorna qualcosa di simile ad una stringa, non lancia eccezioni, ed ha anche una dimensione del codice oggetto accettabile!
std::string_view Progress(double percent)
{
constexpr char bar[] = "oooooooooo........";
constexpr int elements = std::size(bar)/2;
double threshold = percent * elements;
return { &bar[int(elements - threshold)], &bar[int(2 * elements - threshold)] };
}
Bello eh?
Il codice è piccolo, i dati sono meno, fa dei “semplici” calcoli, e risolve i problemi in modo elegante.
Ed è sbagliato, in quanto va fuori dai limiti ogni volta che inserisci un numero negativo.
Possiamo risolverlo? Certo.
std::string_view Progress(double percent)
{
constexpr char bar[] = "oooooooooo.........";
constexpr int elements = 10;
double threshold = std::clamp<double>(percent * elements, 0, elements);
return { &bar[int(elements - threshold)], &bar[int(2 * elements - threshold)] };
}
Vi state già innamorando di questo gioiellino, lo sento.
Quattro linee, due sono dichiarazioni di costanti, sarà certamente facile da mantenere
Giusto?
Sbagliato.
Il codice ha un bug, e neppure ve n’eravate accorti. Il codice corretto è:
std::string_view Progress(double percent)
{
constexpr char bar[] = "oooooooooo..........";
constexpr int elements = 10;
double threshold = std::clamp<double>(percent * elements, 0, elements);
return { &bar[int(elements - threshold)], &bar[int(2 * elements - threshold)] };
}
La differenza è ovvia, vero? No? Beh, l’errore non era in una delle linee super-complicate di sopra, ma nella prima linea. Mancava un punto. E questo avrebbe causato delle letture fuori dal buffer.
E questo non è tutto, perché nel post originale (in inglese) potrete constatare che neppure io sapevo che il codice qui sopra era sbagliato (funzionava sui miei test, ma non su clang). Nel mio modello mentale (errato) assumevo che qualunque compilatore avrebbe evitato di creare una nuova stringa ogni volta, ma avrebbe allocato una costante globale. Questo però non è indicato nello standard (e clang infatti si comporta in maniera diversa, ritornando una string_view che fa riferimento ad una variabile locale, quindi non più valida). Per essere certi di far riferimento ad una costante globale è necessario aggiungere static:
std::string_view Progress(double percent)
{
static constexpr char bar[] = "oooooooooo..........";
constexpr int elements = 10;
double threshold = std::clamp<double>(percent * elements, 0, elements);
return { &bar[int(elements - threshold)], &bar[int(2 * elements - threshold)] };
}
Quindi, neppure la parte “facile” di questo codice era facile da mantenere (neppure per me!). E chi riesce a leggere le linee sottostanti? Ad esempio, perché ho dovuto indicare il parametro template su clamp? Perché double non viene convertito in un intero durante l’accesso all’array? (non cercate, non risponderò a questa domanda qui, questo è un post semplice).
Vediamo le metriche:
Codice: 87 byte
Dati: 21 byte
Chiamate: no
Eccezioni: no
Bene! Molto meglio! Abbiamo vinto!
Ma è davvero il caso di cantar vittoria?
L’esperienza
Volevo chiamare questa sezione “il verdetto”, ma non mi sento di fare il giudice oggi. Anzi, mi sento stupido* per aver riso inizialmente guardando il codice, senza realizzare che si trattava di un modo semplice e relativamente elegante di raggiungere l’obiettivo.
Questo mi ha fatto pensare, e mi sono chiesto:
Il codice originale potrebbe essere mantenuto da uno sviluppatore con poca esperienza? Si.
E il mio codice finale? Probabilmente no. Quindi dovremmo pagare uno sviluppatore esperto per mantenere una barra di progresso.
Il codice originale potrebbe essere scritto senza errori? Si. E qualora ce ne fossero, sarebbero evidenti.
E il mio codice finale? No, o almeno mi pare d’aver dimostrato che non sono in grado di scriverlo correttamente io. Certo, il punto mancante l’ho messo apposta, ma non il secondo bug (infatti ho mantenuto il post originale invariato, aggiungendo una sezione al termine in cui discuto il nuovo bug, trovato da un commentatore).
Alla fine, qual è il costo per mantenere il codice originale, con la catena di if? Probabilmente poco, è intuitivo e facile da leggere.
E il mio codice finale? Secondo me, troppo alto. Se dovessi rileggere quel codice tra una settimana, probabilmente spenderei almeno un paio di minuti a controllare se la chiamata a clamp e la creazione di string_view sono corrette. E questo solo per aver guardato la funzione.
E per ultimo, la domanda più importante di tutte è stata: in qualità di sviluppatore con 25 anni di esperienza in C++, mi piacerebbe mantenere una funzione che visualizza una progress bar, solo perché nessun altro nel mio team potrebbe farlo in modo efficace?
No.
Potrei non cambiare radicalmente il mio stile domani, ma questo esercizio ha cambiato il modo in cui vedrò i “costi di manutenzione” d’ora in avanti, e che terrò più spesso in considerazione questi aspetti in futuro.
* Nota: sono sempre felice di sentirmi stupido, per me significa che sto imparando qualcosa.
#