Come regola, dovresti minimizzare la quantità di logica di business nei costruttori ... Quindi, a seconda di quale sia la funzione / responsabilità della tua classe, puoi scegliere di includere la creazione di socket e il binging del socket a un indirizzo nel costruttore, ma molto probabilmente l'ascolto e la gestione dovrebbero essere eseguiti in metodi separati.
Il costruttore dovrebbe fare solo una cosa e farlo bene: costruisci l'oggetto (qui un'istanza di Server ).
No, non fare tutto questo nel costruttore! Ecco perché
Se aggiungi altre cose, come creare un socket e associarlo a un indirizzo di porta, dovrai affrontare diversi problemi. Ad esempio:
Inoltre, se il costruttore avvia operazioni che non terminano, come ascoltare e gestire le richieste dei client, tutto il codice verrà eseguito su un oggetto non finito (vale a dire prima che il costruttore sia completato).
Infine, alcune lingue non consentono di derivare in modo efficace un nuovo tipo ServerXXX , e sovrascrivono alcuni dei suoi metodi. Ad esempio, in C ++, il costruttore Server verrà eseguito sull'oggetto di base Server di ServerXXX e non sarà a conoscenza dei metodi ServerXXX sottoposti a override. Solo una volta che il costruttore Server sarà finito, passerà al costruttore ServerXXX
Come si fa meglio?
Ci sono diverse alternative. Non conoscendo tutti i dettagli, potrei immaginare di utilizzare un modello di builder (GoF) :
Questo tipo di approccio non è l'ideale per eseguire affari correnti, vale a dire ascoltare e gestire la richiesta. Questo non è sicuramente parte del processo di costruzione. Inoltre, questo potrebbe richiedere il multithreading, un thread che fa l'ascolto e molti altri thread che gestiscono le richieste per i client connessi.
Per questa parte, suggerisco di utilizzare il modello di metodo del modello che implementa l'evento loop per il server. Il codice principale sarebbe quindi qualcosa del tipo:
ServerBuilder builder;
ServerDirector director(builder);
director.construct(); // creates the server and its components
Server server = director.get_result(); // server ready to use
server.mainloop();
Se il tuo costruttore è abbastanza grande che stai pensando di dividerlo in più parti, vorrei tenderci a pensare se questo potrebbe non indicare che la tua classe (e la sua area di responsabilità) è più grande di quanto dovrebbe essere, quindi potresti voler rompere la classe in più pezzi.
Nel caso specifico di un server socket, penso che ci siano davvero due parti (quasi) completamente separate. Uno solo ascolta le connessioni in entrata. È piuttosto generico: quasi tutti i server socket fanno praticamente le stesse cose: creare un socket, collegarlo a un indirizzo, ascoltare le connessioni in entrata. Una volta che accetta una connessione, la consegna ad un'altra classe che ha tutte le logiche di business per gestire le interazioni con il client.
Questa seconda classe è un gestore di sessioni che riceve solo un socket aperto che è già stato accettato e comunica con il client tramite il socket. È più difficile dire esattamente cosa succede qui, perché la maggior parte dipende dall'interazione tra client e server.
Quando ho scritto un codice come questo, nessuna delle due classi ha avuto un costruttore così grande o complesso che ho visto molto (qualsiasi, davvero) motivo per romperlo in pezzi. L'ascoltatore fondamentalmente fa socket() , bind() e listen() . Qualcosa come una dozzina di righe di codice - e tre quarti di questo è la gestione degli errori e così via (anche se è necessario gestire qualcosa come SSL / TLS, la complessità cresce un po ').
Preferisco i costruttori che restituiscono istanze di lavoro della classe data. (Analogia del mondo reale: se ordini un'auto, ti aspetti che funzioni, nel senso che puoi scappare con esso, ed è quello che mi aspetto da un costruttore).
Naturalmente, la definizione di "lavoro" è specifica dell'applicazione. Quindi la domanda è: cosa ti aspetti da un Server ?
Server . Server se non ascolta su nessuna porta? Server se non gestisce le richieste dei client? In genere, risponderei a tutte queste domande con un SÌ e fare in modo che il costruttore esegua tutte queste operazioni (non creando un costruttore di 100 righe, ma suddividendo le azioni in singoli metodi, ovviamente). Ma dopo che il server è stato costruito, serve.
Forse le tue risposte sono diverse, o forse Server è pensato per essere una classe base astratta - quindi il costruttore dovrebbe essere diverso, ovviamente.
Altre risposte coprono molto bene. Una cosa non menzionata è il modo in cui funzionano le interfacce IDisposable (.NET) e AutoClosable and Closable (Java).
È buona norma in .NET utilizzare l'istruzione using e provare con risorse ove possibile.
Se inseriamo la logica nel costruttore questo non sarebbe possibile. Quindi questo è un altro "con" per la lista dei professionisti e dei contro.
Supponendo che tu abbia una classe che, tra le altre cose, gestisce il bind iniziale della porta e imposta la gestione degli eventi come un must (ovvero deve assolutamente essere eseguito prima che qualsiasi altra operazione possa essere eseguita), un modo in cui potresti farlo è per creare un metodo pubblico init che deve essere chiamato prima di ogni altro. ... Ma questo è brutto. Più precisamente, richiede che il chiamante prenda delle considerazioni che dovrebbero essere oscurate dalla classe generale stessa.
Una soluzione migliore sarebbe quella di inizializzare pigramente il socket e la configurazione della gestione degli eventi. Vale a dire, hai un'istanza di Server con un membro booleano privato isInitialized inizialmente impostato su falso. Prima di chiamare qualsiasi metodo, esegui questo controllo ed esegui la chiamata necessaria al metodo privato init , quindi imposta isInitialized a true. Se hai pochi metodi pubblici, questo è l'ideale e sporca la tua implementazione reale molto poco. Se hai molti metodi pubblici, dovresti prendere in considerazione la creazione di una seconda classe che si occupa del nocciolo della connessione, che usi per eseguire le azioni di basso livello, incluso stabilire quando la connessione è formata (tutto ciò sarebbe oscurato dal chiamante a Server ).
Sebbene sia vero che non creerai un'istanza Server senza usarla (quindi il beneficio secondario del caricamento lento non si applica in realtà), ti fornisce comunque un mezzo per rimuovere il codice potenzialmente esplosivo all'interno il tuo costruttore. Anche l'errore verrebbe più gettato in un punto più intuitivo, in un metodo chiamato init all'interno del metodo send , che è probabilmente un luogo che verrà coperto dai blocchi di prova try comunque.
Certamente preferisco rompere i grandi costruttori, con un avvertimento:
Userei solo static , preferibilmente puro, metodi per il lavoro. Ciò riduce la necessità di preoccuparsi di chiamare i metodi di istanza su un'istanza incoerente.
Ad esempio, potrei avere qualcosa di simile:
class MyClass {
private:
Socket socket;
public:
MyClass(Address address, Port port) {
this->socket = MyClass::createSocket();
MyClass::bindSocket(this->socket, address, port);
// etc
}
Socket createSocket() const {
return Socket(); // hypothetical. Add socket construction logic here
}
void bindSocket(Socket socket, Address address, Port port) {
socket.bind(address, port); //hypothetical
}
};
Leggi altre domande sui tag clean-code