Questo codice ha solo un livello di astrazione?

Sono uno studente di Clean Code me stesso. Ecco il mio tentativo allo stile dello zio Bob.

var takeTurn = function(line, column) {
  if( moveIsValid(line, column) ) {
    move(line, column);
    verifyWinner();
    changePlayer();
  }
  else {
    changeStatusMessage("Invalid Move!");
  }
}

Per prima cosa dobbiamo decidere su quale livello di astrazione stiamo lavorando. La maggior parte del lavoro qui non riguarda la mossa, ma la presa di turno di base. La condizione if ci lega ai dettagli delle regole di mossa e non si trasformano prendendo così l'ho astratto con una funzione.

Quella libera è questa funzione di altri problemi di movimento che potrebbero essere aggiunti come un controllo contro lo spostamento di una regina come un cavaliere, un controllo se l'utente sta tentando illegalmente di catturare i propri pezzi, spostando diagonalmente una pedina che non sta catturando , arrocco dopo che il re si è mosso, ecc.

È ora a un livello di astrazione? Ben aggiornare il tabellone dopo una mossa non riguarda la presa di turno. Quindi ho spinto drawBoard() in makeMoveInPosition()

verifyWinner() e changePlayer() riguardano la presa di turno. Ad esempio, cosa succede se decidi la conseguenza per aver fatto una mossa non valida è perdere un turno o perdere il gioco? A volte gli scacchi vengono giocati in questo modo. Quindi penso che sia bello averli qui dove possono essere spostati a volontà.

Ho anche ribattezzato la funzione in modo che chiunque guardi dentro la funzione per la prima volta non si sorprenda che sia pieno di logica di presa e non di spostamento della logica. Ciò segue qualcosa chiamato il principio del minimo stupore . È bello quando si guarda in una funzione e fa più o meno quello che ti aspettavi. Il modo migliore per favorire questo è dare alla funzione un buon nome ed essere fedeli al suo significato.

Livello di astrazione

Considererei l'istruzione if nella riga 1

if( board[line][column] === EMPTY) 

e la chiamata changeStatusMessage ()

changeStatusmessage("Invalid Move!")

come livello di astrazione più basso degli altri. Gli altri sono piuttosto astratti, non ci sono dettagli di implementazione visibili. Ma le due linee date sono piuttosto di basso livello. Il mio consiglio per il primo refactoring sarebbe

if( boardIsEmpty(line, column) )

e

showInvalidMoveError()

Fai una cosa

Per quanto tu stia facendo, direi che stai facendo troppo. cerchi una tavola vuota e poi esegui uno spostamento oppure mostra un errore . L'esecuzione tuttavia è suddivisa in diversi passaggi, che in questo senso potrebbero anche essere un diverso tipo di astrazione. Quindi considera un altro refactoring:

if( isValidMove(line, column) )
    executeMove(line, column);
else
    showInvalidMoveError();

Si potrebbe anche sostenere che è ancora molto da fare, dato che anche gestisce un errore .

Quindi, probabilmente, avrei dovuto solo un errore come questo

else throw Error(INVALID_MOVE);