Numeri magici, località e leggibilità

In parole povere:

• Se estrai il channelID in più punti, dovresti creare una funzione.
• Tale funzione, essendo coesa, dovrebbe leggere dallo stato.
• Parte di quello stato sarebbe la costante con un nome come DELIMITERS_BEFORE_ID .

Questo lo trovo:

String channelId = channelIDdWithPath.split("/")[DELIMS_BEFORE_ID];

... è più leggibile che questo

String channelId = channelIDWithPath.split("/")[3];

.. questo è ancora meglio:

String channelId = channelIDWithPath.split(PATH_DELIM)[DELIMS_BEFORE_ID];

.. ma è meglio:

String channelId = extractChannelID(channelIDWithPath);

• Non è necessario verificare se il valore assegnato alle costanti o alle variabili di classe sono corretti ogni volta che si legge il codice. L'idea è che se le cose superano il test, puoi "astrarti" dal loro funzionamento interno. Il tuo cervello può gestire solo tanta complessità, quindi dovrai, col tempo, smettere di fare il tifo se il valore di DELIMS_BEFORE_ID è OK e iniziare a usare i pezzi lego senza fare attenzione se i blocchi lego contengono o meno la giusta quantità di acrilonitrile butadiene stirene.

Dovresti concentrarti su ciò che rende il tuo codice leggibile e lo spirito delle linee guida piuttosto che provare a dimostrare che puoi scrivere codice cattivo anche seguendo le linee guida.

Una costante denominata NUMBER_OF_DELIMITERS_BEFORE_ID_IN_CHAT_CHANNEL_PATH non è molto chiaro o comprensibile. Quindi sì, un numero magico non è probabilmente peggiore dell'uso di una costante così male nominata. Ma questo non è un argomento per usare numeri magici, questo è un argomento per usare nomi migliori per le tue costanti, o per riscrivere il codice per usare un approccio diverso.

Il tuo primo esempio di codice mostra un modo semplice per ottenere il terzo segmento. Ma presumibilmente hai bisogno anche degli altri segmenti di URL da qualche altra parte nell'app, quindi devi ripetere .split("/")[index] e il commento di accompagnamento ogni volta che ti serve un segmento. Questo interrompe il principio "non ripetere te stesso" e improvvisamente la logica di analisi degli URL è diffusa in tutta l'applicazione. Se lo schema dell'URL cambia (ad esempio, se viene introdotto un nuovo segmento tra il secondo e il terzo), devi aggiornare molti numeri e commenti, che sono soggetti a errori.

A proposito, non dovresti avere 100 righe di codice non correlato in un metodo o in una classe. Secondo il principio della responsabilità unica, tutto in una classe dovrebbe essere correlato, altrimenti dovrebbe essere suddiviso. Una variabile o una costante dovrebbe sempre essere dichiarata il più vicino possibile al suo utilizzo e nel minor spazio possibile.

Se hai la costante a livello di classe, ti suggeriamo di usarla in più punti per estrarre segmenti dal percorso. La soluzione qui è di incapsulare l'analisi dell'URL in un unico posto, in modo da utilizzare solo gli indici lì, e non esporli al programma in generale. Di nuovo il problema non è costanti contro letterali, ma incapsulamento.

Vedi il commento valido di Luc sulla tua domanda. Modelli di URL. Vorrei aggiungere ... ogni volta che vuoi un numero magico che non sia né 0 né 1 e non si riferisca nemmeno a una specifica funzione indipendente del dominio aziendale, cioè all'odore del codice.

Il tuo problema qui non sta nel fatto che stai codificando un numero magico tanto quanto stai codificando qualcosa che sta replicando informazioni che già esistono implicitamente. Se i tuoi modelli di URL cambiano, il tuo numero magico ora deve essere sincronizzato. Questo è un problema di manutenzione (minore), e questi si sommano e rendono più probabili gli errori.

OK ci sono due domande in competizione qui. Uno è qual è il modo "migliore" "ufficiale" per programmare questo e Due è in realtà più leggibile

quindi ecco il mio modo "ufficiale" migliore

public class ChannelInfo
{
    public String ChannelId { get; set; }
}

public class PathExtractor : IPathExtractor
{
    private const int NUMBER_OF_DELIMITERS_BEFORE_ID_IN_CHAT_CHANNEL_PATH = 3;
    private const char SPLITCHAR = '/';
    public ChannelInfo GetChannelInfo(string path)
    {
        var info = new ChannelInfo();
        info.ChannelId = path.Split(new char[] { SPLITCHAR })[NUMBER_OF_DELIMITERS_BEFORE_ID_IN_CHAT_CHANNEL_PATH];
        return info;
    }
}

Ora, è "migliore" e "più leggibile (tm)" di

/* Extract id from "/toClient/chat/id". */
String channelId = channelIdWithPath.split("/")[3];

Dico NO !! per favore! controlla il tuo shock e la rabbia caro lettore, mentre aggiungo alcune condizioni ....

• Se utilizzi il codice in molti punti, il primo metodo potrebbe salvarti alcuni caratteri

• Se hai più implementazioni di IPathExtractor il primo metodo è più flessibile

• Se disponi di più proprietà che puoi aggiungere a ChannelInfo, il primo metodo diventa più facile da usare

Quindi, in generale, sì è probabilmente una buona idea dividere la logica in classi ecc.

MA

• Se hai un solo punto in cui estrai un singolo valore come parte di una funzione più grande e viene commentato

Quindi no, non è particolarmente più leggibile o più facile se lo dividi in un'interfaccia più un'implementazione più un oggetto di ritorno, oltre a tutte le proprietà e i metodi ecc.

Avrai aggiunto una tonnellata di codice in più solo per poter dire che stai selezionando "clean coding" e spuntando le caselle di controllo dello stile