I have a hard time understanding when the Single Responsibility Principle is correctly used. Consider the following code:// --------------------------------------------------------------------------------------------------void CTCPClient::Try_Send_Data(const char *Outgoing_Message)// --------------------------------------------------------------------------------------------------{    Reset_Return_Value();    Copy_Data_Into_Buffer(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);    }}If I would read this method for the first time, I would, as the name sugest, expect a try and catch statement. But before that Im calling two additional methods (Reset_Return_Value(), Copy_Data_Into_Buffer()). Is the SRP already violated here?Further in the method I'm setting Keep_Alive_Counter = 0;. In my opinion, at least at this point the SRP has been violated in addition to the fact, that if I were to be someone who didn't write the code, would simply not expect this line to be there. The problem I have here is: If I do this in an explicit method, it will be a one line method, something I encounter more often, as I try to consinder the SRP. These methods still look weird to me and I haven't found an answer to when they are considered good pratice. Always? In some cases? Never?This is my own code so feel free to criticize, as long as it is constructive.EDIT: Keep_Alive_Counter and Return_Value are class attributes, which are changed/used by other methods.
Problem with understanding SRP
design;design patterns;c++;single responsibility
When considering whether or not a function or a method violates the SRP, you should look not at the implementation of the method but at what it's supposed/promises to do. Consider the following two functions:void printTime() {    print(getTime());    invade(POLAND);}void printTimeAndInvadePoland() {    print(getTime());    invade(POLAND);}Only the second function violates the SRP. printTimeAndInvadePoland has two responsibilities - to print the time and to invade Poland. printTime does not violate the SRP, as it has only one responsibility - to print the time. It has a different problem - unwanted side-effects. You just want to print the time and suddenly you started a world war. printTimeAndInvadePoland does not have unwanted side-effects because it's supposed to invade Poland - that side-effect is part of it's responsibility.Let's look at your Try_Send_Data. It has one responsibility - to send the data. You are worried about the calls to Reset_Return_Value and Copy_Data_Into_Buffer and about resetting Keep_Alive_Counter. Does any of them violate the SRP? No - because they don't change the fact that Try_Send_Data's only responsibility is to send the data. So the question is - do they make the method deviate from it's responsibility?Reset_Return_Value(); - it looks like it refers to the return value of some lower level data transmission function that you call from Send_Data, and Send_Data is setting. This is not a violation of the SRP, but it's still a bad practice: Send_Data should either return that value or throw an exception if it indicates an error. Even if you keep Send_Data as is, if it guarantees to always set Return_Value you can move the if(Return_Value <  0) check to only happen when Send_Data is called, and then you won't have to reset it.Copy_Data_Into_Buffer(Outgoing_Message); - I see no problem here. While I don't know if it copies the data to or from Outgoing_Message, this copy seems like a necessary task for sending the data, which supports Try_Send_Data one responsibility.Keep_Alive_Counter = 0; - updating the keep-alive information is part of the data sending process, so there are no weird side-effects here, but I don't think this line belongs here. If you update the keep-alive info every time you send data(makes sense), it should be done inside Send_Data. If, like the comment indicates, you only update it when a button is pressed(WTF?), it should be done in the button press event handler.Of course, if Send_Data is not yours, and Try_Send_Data is a convenience wrapper around Send_Data's awkward API, this may be acceptable - though you might want to rename Try_Send_Data, because right now it's name indicates it's just a variation of Send_Data that does not throw, when it clearly does more. If you can change Send_Data, consider moving some of Try_Send_Data's functionality(like copying the data to the buffer, resetting the keep alive counter, and throwing in case of bad return value) to Send_Data.