First, it's worth noting that the example in the article somewhat contrived (for practical reasons), and that context matters when it comes to these things. E.g. if you're writing a small, one-off tool, there's little reason to bother too much with design. But let's say this is a part of some longer term project, and that you can reasonably expect that this code would benefit from some design changes (or that you've already had to implement changes that clash with the current design), and let's examine it in that context.
Here's the code for reference:
public static string GetTimeOfDay()
DateTime time = DateTime.Now;
if (time.Hour >= 0 && time.Hour < 6)
if (time.Hour >= 6 && time.Hour < 12)
if (time.Hour >= 12 && time.Hour < 18)
In C#, the
static keyword essentially means that this is a free function (i.e., not an instance method on an object). This is relevant within the context of your question, since you ask how these considerations apply to objects.
The author of the article raises several points; let me first address 1. (tightly coupled to the date-providing service - the
DateTime class) and 3. (misleads about dependencies). The problem this creates is that, while the function works well under the circumstances it was originally created for, is not usable in other contexts.
E.g., what if I need to support a UI that allows users to see the "time of day" category for some future date (again, this "Morning/Afternoon/Evening/Night" example is contrived, but suppose it returns some business-relevant category instead, something of interest to the users).
Another such context is, of course, testing, where you want to be able to plug in predefined values (currently not possible) and check for results (from the perspective of a test, the function is non-deterministic - you can't tell what to expect).
This is easily fixed by making the date-time be a parameter:
public static string GetTimeOfDay(DateTime dateTime)
// same code, except that it uses the dateTime param...
Now, regarding SRP violation (point 2.) - the problem is, it's not very meaningful to talk about it in abstract terms. What I mean by that is that it's not very meaningful to just look at the code in isolation and consider a bunch of "what if" scenarios. Sure, there are some general things you can say about SRP in this way, but if you don't consider how your code is actually changing, and the actual design needs, you'll end up with a loot of wasted effort and with overly complicated (read "over-engineered") code.
This means that while you can and should apply SRP initially based on a couple of educated guesses and reasonable assumptions, you'll have to reconsider your design over several iterations/sprints as your understanding of the actual responsibilities and change patterns increases, as you work on this code.
Now, the author says that the function "consumes the information and also processes it". That's too vague to be useful, you could say that about any function. And even if a function delegates the processing to lower level code, at the end of the chain, there has to be something that "consumes the information and also processes it".
The thing is, if this part of the codebase changes very rarely (or never), then you don't really need to consider SRP. You could come up with any number of different reasons to change, but if those changes never happen, you've paid the design costs without getting any benefits. E.g., perhaps the strings returned should be available in different languages (maybe the function should return a key to some dictionary to support localization). Or maybe the threshold values for different times of day can vary - maybe they should be read from a database. Or maybe these values change throughout the year. Or maybe this entire logic isn't universal, so maybe some sort of strategy should be injected into the function (the Strategy pattern). What about a design that needs to support all of the above?
See what I mean by a bunch of "what if" scenarios? What you should do instead is develop an understanding of the problem domain and the codebase, and apply SRP so that the most prominent change axes (kinds of changes, responsibilities) are well supported.
The concept of a seam
So, when you design functions or classes (or libraries and frameworks, for that matter), you often provide some extensibility points - places where client code can plug something in, or otherwise parametrize the provided behavior. Michael Feathers (in Working Effectively with Legacy Code) calls these "seams" - a seam is a place where you can join two software components together. Making datetime be a parameter is a very simple seam. Dependency injection is also a way to create seams. E.g., you could also inject a function or an object that can return a datetime instance (this may or may not be an overkill in the context of this particular example).
What about objects?
So far, we've been considering things at the level of a free function; objects provide another organizational level. So you now have to consider the object as a whole, as objects have their own mechanisms for introducing seams.
The typical way to do so is through constructor injection (as this results in a ready-to-use object)1. A (Python) class that's equivalent to the example code above would be:
self.datetime = datetime; # from datetime import datetime
now = self.datetime.now()
if 0 <= now.hour < 6:
if 6 <= now.hour < 12:
if 12 <= now.hour < 18:
This has the same issues, but the problem now isn't the method itself, it's the fact that the class constructor creates the datetime dependency internally, and it doesn't offer an explicit way to plug in something else. There's no built in seam for this purpose. It's not easy to reuse the class a different scenario.
Here's the same class, but now the constructor takes a "datetime provider":
def __init__(self, datetimeProvider):
self.datetimeProvider = datetimeProvider;
now = self.datetimeProvider.now()
if 0 <= now.hour < 6:
if 6 <= now.hour < 12:
if 12 <= now.hour < 18:
dts = DateTimeServices(datetime)
Now you can plug in different things, as long as the thing that plays the role of
datetimeProvider satisfies the required interface (which, in this case, consists only of the now() method that returns a datetime instance). E.g.:
def __init__(self, year, month, day, hour, minute = 0, second = 0):
self.datetime = datetime(year, month, day, hour, minute, second)
dts = DateTimeServices(FakeDateTimeProvider(2020, 8, 18, 8))
# always returns "Morning"
This addresses concerns 1. & 3. from before (with the same considerations regarding concern 2. (SRP)). So, you see, the use of
self isn't the problem in itself, it has more to do with the design of the class. As other answers have mentioned, when you use a class (or more precisely, an object), you know what that object represents conceptually, and it isn't surprising to you, the programmer, that the class has and uses its internal state.
def __init__(self, power):
self.power = power
self.speed = 0
def accelerate(self, acceleration_time):
self.speed = self.calculate_acceleration(acceleration_time, self.power)
From my understanding of the class Car, from the naming of the method, and perhaps from documentation, it isn't surprising to me that
accelerate changes the state of the instance. This is not something unexpected for objects.
What's problematic is if the class has hidden dependencies that are somehow relevant to your work, making things harder for you.
That said, what can be confusing (in light of the above) is that often instance methods need to take their own parameters. Think of these as accepting additional contextual information that's not directly related to the core responsibility of the class. E.g., it's not something that you can pass once to the constructor, but something that may change on every call. One classic toy example is shapes (circles, triangles, rectangles) that can draw themselves (or, instead of shapes, these could be UI elements (buttons, labels, etc), or game entities (say, 2D sprites)). One way to do it is to have a parameterless draw() method, that does all the drawing internally. But what if you want to draw the same thing in a completely different part of a UI, on a separate drawing surface? Or on a different buffer so that you can do special effects like portals or mirrors? The more flexible alternative is to pass in the drawing surface (or some kind of a graphics object) as a parameter to the draw method.
but what about methods with hundred of lines which reads and assigns to self in many places?
Take that code and burn it with fire.
If those are not named properly developer will have big troubles to understand what it does and even if those are named properly developer should read whole implementation to know if it does modify some self stuff, or if additional context is injected with self.
Yeah. Exactly. Don't write methods with hundreds of lines of code.
Now, on a more serious note, sometimes, you'll end up with large methods. But most of the time, strive to decompose your code into smaller methods and small classes.
If you do have a large method like the one you're describing, one that you can't make the heads or tails of, that methods suffers from all kinds of design problems that you're not going to solve by changing its signature. It's not about
self, or about what parameter it takes - this method has bigger issues. You have to refactor it, find things that are generalizable, and break it down into smaller, more understandable and more dependable chunks (methods that you don't have to look into in order to understand the method that calls them). You may even end up putting those chunks in completely different classes.
On the other hand when I'll try to code every method without using self, with input (arguments) and output (return value) then I'll end up passing one variable through many methods and I'll repeat myself.
Well, don't go to either extreme. Write relatively small classes, try to find useful abstractions, and be deliberate about what you pass in as a parameter/dependency of the object itself, and what you want to provide as contextual information to individual methods. Consider if instances of your class should appear in scenarios other than the one you originally intended, and see if your design can accommodate them.
How to make it clear what method uses as input and what it modifies (output)?
Again, when it comes to objects, what you want is to do is make clear what the object itself represents. For object level dependencies, use (preferably) constructor injection and make it clear what the class represents conceptually, what it does, and how it's meant to be used. For instance methods, use good naming, describe what they do and make use of contextual parameters when required. As for class methods and static methods, threat them more as free functions that are somehow closely related to the concept represented by the containing class (these are often things like helper methods and factories).
1 Sometimes constructor injection is not feasible (e.g. a framework may require a parameterless constructor), so dependencies are injected via methods or properties instead, but this is less ideal.