La creazione di "metodi di scelta rapida" nella superclasse di un controller Bad Practice?

Is creating methods such as isUserAdmin() bad practice?

È, anche se non è tanto che questo metodo sia una cattiva pratica, ma piuttosto che usare una classe genitore come una discarica per i metodi che vuoi condividere con gli oggetti figli ma che non hanno alcuna relazione particolare con l'un l'altro in termini di comportamento è una cattiva idea.

Se il metodo era una funzionalità di base di tutti i controller, allora metterlo nella classe genitore è una buona idea. Ma se è solo un metodo di utilità che alcuni controller potrebbero utilizzare, la classe genitore non è il posto giusto.

Non vuoi che il tuo genitore si riempia di metodi che i bambini potrebbero o non potrebbero aver bisogno a seconda di cosa fanno. Un gruppo di classi figlio potrebbe utilizzare questo metodo isUserAdmin, ma molte potrebbero non farlo.

Mantenere la classe controller piccola e contenente metodi specifici che verranno utilizzati da tutti i controller. Se hai un comportamento che alcuni controller potrebbero utilizzare, usa questo composizione piuttosto che ereditarietà . L'ereditarietà è solo per il comportamento condiviso da tutti i bambini.

Se hai alcuni controller che devono cercare se l'utente è un amministratore e non vuoi reimplementare questo codice in ogni oggetto (che sei corretto non voler fare), crea un oggetto che contiene questo comportamento che puoi dare ai controllori quando vengono creati.

Due punti:

1. Preferisco i metodi come questi perché aiutano a scomporre i treni di metodo come: myObject.getFoo().doBar().tooManyCalls() (cioè Train Wrecks)
2. Tuttavia, se ti ritrovi a scrivere molti di questi metodi di "scorciatoia", puoi benissimo avere un refactoring più grande / migliore di cui puoi trarre vantaggio.

Vedi il libro di Bob Martin "Clean Code" e code-smell G36

Usi il buon senso.

Mi potrebbe interessare molto sapere se l'utente è un amministratore e aggiungere un metodo per questo. Potrei preoccuparmi molto dell'oggetto di servizio utente del controller e utilizzarlo molto, per tutti gli scopi per cui è stato progettato un oggetto del servizio utente. In tal caso, aggiungerei un metodo per l'oggetto servizio utente.

Ho il sospetto che non mi interessa molto l'oggetto contenitore di servizi e che si tratta solo di dettagli di implementazione. In questo caso, dover chiamare getServiceContainer (). GetUserService () invece di getUserService sarebbe molto fastidioso. Soprattutto se il progettista della classe non voleva davvero che questo fosse più di un dettaglio di implementazione, e avere l'accesso a getServiceContainer () ovunque nel mio codice rende molto difficile cambiare.

A quanto pare non vuoi correggere questa catena di metodi quando isUserAdmin() cambia la sua posizione. Quindi, rispetto alla Legge di Demetra, l'hai spostata nella classe base, il che suona ragionevole. Quindi quello che hai ora è riutilizzo del codice con ereditarietà , che è, sì, una cattiva pratica. Quindi ti riconduce al punto in cui hai iniziato.

Come soluzione vedo iniettare un utente stesso nel controller. Anche se potrebbe essere imbarazzante, dal momento che molto probabilmente stai usando un framework con DI-container. Qui è dove un puro DI sta uscendo.