Does having a method do more than one thing violate the Single Responsibility Principle?

Question

For my purposes, I need to search for a specific node in an xml file and, if found, delete it. Should I pull search functionality out into its own method and delete functionality out into its own method? It seems more expensive to do it this way because I'll be searching the xml file once to see if it exists and searching it again to delete it. If I combine these two functionalities into a single method I can delete it right when I find it. Am I understanding SRP correctly here?

Answer 1

Do you have any other reasons/situations in which you are searching the xml file? In general it is a Good Thing to separate distinct jobs at any level, regardless of adhering to or violating someone's rule (that's my rule ;-) ). Separating these functions might (?) also make your code more understandable, which may turn out to be more important than a trivial gain in performance.

Answer 2

Your average XML Parser will create Nodes which know their Parents so you can do something like:

XmlNode node = this.FindNode(filter);
node.ParentNode.DeleteChild(node);

This way you have split both functions but no overhead.

Regarding the core of your Question: Yes, searching and deleting in one Method violates the single responsibility but performance and SRP don't mix that well in many cases so you have to decide whats more important.

PS:
Example is not (knowingly) related to any real language out there.

Answer 3

No, the Single Responsibility Principle isn't about the details of how code is written. It's about how to divide up the functionality of a program into classes. It says that if a class is likely to change for more than one reason, it should be two classes. A classic example is a class that constructs and formats a report; the contents of the report and the format of the report are likely to change at different times, so that class is a good candidate for refactoring into two.

You don't say what the functional responsibility of your class is, but, from the point of view of whatever job your class is supposed to get done, searching for and deleting the XML node are just parts of that single job, and doing them in the same class and in one operation does not violate the SRP.

(On the other hand, if your class had a lot of domain logic and also a lot of nuts and bolts about manipulating XML, it would violate the SRP.)

Answer 4

It does violate Command Query Separation Principle which I feel goes hand-in-hand with SRP. Searching and deleting are two things can change so those could also be defined as two separate responsibilities. They can be unit tested separately, you may have a bug in how you find the node, but not in the deletion. You may also want to mock out the deletion part. It also gives you an intermediate point in between the finding and deleting (again this goes back to unit testing and debugging).

All in all I'd say there are lots of benefits to command query separation so I try to follow it whenever possible.

Don't prematurely optimize your code! Write it in the most maintainable way / best design you can, then IF it is a bottle neck you can tweak it.