Problema con la comprensione di SRP

Quando si considera se una funzione o un metodo violano l'SRP, non si dovrebbe guardare l'implementazione del metodo ma a cosa si suppone / si promette di fare. Considera le seguenti due funzioni:

void printTime() {
    print(getTime());
    invade(POLAND);
}

void printTimeAndInvadePoland() {
    print(getTime());
    invade(POLAND);
}

Solo la seconda funzione viola l'SRP. printTimeAndInvadePoland ha due responsabilità: stampare l'ora e invadere la Polonia. printTime non viola l'SRP, in quanto ha una sola responsabilità: stampare l'ora. Ha un altro problema: effetti collaterali indesiderati. Vuoi solo stampare l'ora e improvvisamente hai iniziato una guerra mondiale. printTimeAndInvadePoland non ha effetti collaterali indesiderati perché dovrebbe invadere la Polonia - quell'effetto collaterale fa parte della sua responsabilità.

Diamo un'occhiata al tuo Try_Send_Data . Ha una sola responsabilità: inviare i dati. Sei preoccupato per le chiamate a Reset_Return_Value e Copy_Data_Into_Buffer e al ripristino di Keep_Alive_Counter . Qualcuno di loro viola l'SRP? No - perché non cambiano il fatto che l'unica responsabilità di Try_Send_Data è quella di inviare i dati. Quindi la domanda è: fanno deviare il metodo dalla sua responsabilità?

• Reset_Return_Value(); - sembra che si riferisca al valore di ritorno di una funzione di trasmissione dati di livello inferiore chiamata da Send_Data e Send_Data è impostata. Questa non è una violazione dell'SRP, ma è ancora una cattiva pratica: Send_Data dovrebbe restituire quel valore o generare un'eccezione se indica un errore. Anche se si mantiene Send_Data così com'è, se garantisce di impostare sempre Return_Value , è possibile spostare il controllo if(Return_Value < 0) affinché si verifichi solo quando viene chiamato Send_Data , e quindi non sarà necessario ripristinarlo.

• Copy_Data_Into_Buffer(Outgoing_Message); - Non vedo alcun problema qui. Sebbene non sappia se copia i dati in o di Outgoing_Message , questa copia sembra un'attività necessaria per l'invio dei dati, che supporta Try_Send_Data one responsabilità.

• Keep_Alive_Counter = 0; - l'aggiornamento delle informazioni keep-alive fa parte del processo di invio dei dati, quindi non ci sono effetti collaterali strani qui, ma non penso che questa linea appartenga a questo. Se aggiorni le informazioni keep-alive ogni volta che invii i dati (ha senso), dovrebbe essere fatto all'interno di Send_Data . Se, come il commento indica, lo si aggiorna solo quando si preme un pulsante (WTF?), Dovrebbe essere fatto nel gestore eventi del pulsante press.

Ovviamente, se Send_Data non è il tuo e Try_Send_Data è un comodo wrapper attorno all'API awkward di Send_Data , questo può essere accettabile, anche se potresti voler rinominare Try_Send_Data , perché al momento è il nome indica che è solo una variazione di Send_Data che non genera, quando fa chiaramente di più. Se puoi modificare Send_Data , prendi in considerazione la possibilità di spostare alcune delle funzionalità di Try_Send_Data (come copiare i dati nel buffer, reimpostare il contatore keep alive e generare un valore di ritorno errato) a Send_Data .

La risposta alla tua domanda è molto semplice.

Martin [who coind the word] defines a responsibility as a reason to change, and concludes that a class or module should have one, and only one, reason to change. (source)

Ciò significa che se la tua funzione, indipendentemente dal loro nome o da quello che promettono di fare, ha più di una ragione per cambiare, allora viola l'SRP.

Ad esempio, una funzione che cerca un record utente nel database e restituisce il risultato, non viola l'SRP, anche se potrebbe avere una logica lunga e complicata all'interno.

D'altra parte, una funzione che cercherebbe l'utente e restituirebbe il record dell'utente o crearne uno se non riuscisse a trovare alcun record corrispondente, viola SRP perché la seconda funzione ha due ragioni per la modifica 1) quando il cercare le modifiche logiche 2) quando cambia la logica di creazione del record.

Il tuo codice è piuttosto strano, perché stai controllando variabili non definite in quel metodo. Non vedo l'intera classe, ma posso solo supporre che siano in qualche modo cambiati in altri metodi. Quindi, anche se hai cercato di renderlo bello, sei riuscito a creare qualcosa che assomiglia al codice spaghetti.

Per rispondere alla tua domanda: sì, questo è SRP, ma fatto in pessimo modo.

Posso consigliarti di leggere "Clean Code" , e rifare la tua classe.

Penso che SRP non sia usato correttamente qui. Reset_Return_Value () e Copy_Data_Into_Buffer () possono essere utilizzati separatamente. È possibile impostare una variabile per contenere i valori e utilizzare questi due metodi separatamente. In questo modo, se il meccanismo di Reset_Return_Value () o Copy_Data_Into_Buffer () cambia, non avrà effetto sul codice. Qualcosa come il seguente:

Reset_Return_Value();
Copy_Data_Into_Buffer(Outgoing_Message);
Try_Send_Data(Outgoing_Message);
// -------------------------------------------------------------------------  -------------------------
void CTCPClient::Try_Send_Data(const char *Outgoing_Message)
// --------------------------------------------------------------------------------------------------
{
  try
   {
    if((strcmp(Outgoing_Message, "") != 0))
    {
        Send_Data();
        Keep_Alive_Counter = 0; // reset as a button has been pressed
    }

    if(Return_Value <  0)
    {
        throw SEND_DATA_TCP_ERROR;
    }
}
catch(EError_ID)
{
    Error_Message = strerror(errno);
    LOG_WARNING(__SOURCE__ << Error_Message);
 }
}
// --------------------------------------------------------------------------------------------------
void CTCPClient::Reset_Return_Value()
// --------------------------------------------------------------------------------------------------
{
           //...code to reset the return value
}
// --------------------------------------------------------------------------------------------------
void CTCPClient::Copy_Data_Into_Buffer(const char *Outgoing_Message){
// --------------------------------------------------------------------------------------------------
{
              //.........code to copy the data into buffer
}