My colleagues like to say "logging/caching/etc. is a cross-cutting concern" and then proceed using the corresponding singleton everywhere. Yet they love IoC and DI.
Is it really a valid excuse to break the SOLID principle?
Software Engineering Stack Exchange is a question and answer site for professionals, academics, and students working within the systems development life cycle. It only takes a minute to sign up.
Sign up to join this communityMy colleagues like to say "logging/caching/etc. is a cross-cutting concern" and then proceed using the corresponding singleton everywhere. Yet they love IoC and DI.
Is it really a valid excuse to break the SOLID principle?
No.
SOLID exists as guidelines to account for inevitable change. Are you really never going to change your logging library, or target, or filtering, or formatting, or...? Are you really not going to change your caching library, or target, or strategy, or scoping, or...?
Of course you are. At the very least, you're going to want to mock these things in a sane way to isolate them for testing. And if you're going to want to isolate them for testing, you're likely going to run into a business reason where you want to isolate them for real life reasons.
And you'll then get the argument that the logger itself will handle the change. "Oh, if the target/filtering/formatting/strategy changes, then we'll just change the config!" That is garbage. Not only do you now have a God Object that handles all of these things, you're writing your code in XML (or similar) where you don't get static analysis, you don't get compile time errors, and you don't really get effective unit tests.
Are there cases to violate SOLID guidelines? Absolutely. Sometimes things won't change (without requiring a full rewrite anyways). Sometimes a slight violation of LSP is the cleanest solution. Sometimes making an isolated interface provides no value.
But logging and caching (and other ubiquitous cross-cutting concerns) aren't those cases. They're usually great examples of the coupling and design problems you get when you ignore the guidelines.
String
or Int32
or even List
out of your module. There's just an extent to which it is reasonable and sane to plan for change. And, beyond the mostly-obvious "core" types, discerning what you might likely change is truly just a matter of experience and judgement.
Yes
This is the whole point of the term "cross-cutting concern" - it means something that does not fit neatly in the SOLID principle.
This is where idealism meets up with reality.
People semi-new to SOLID and cross-cutting often run into this mental challenge. It's OK, don't freak out. Strive to put everything into terms of SOLID, but there are a few places like logging and caching where SOLID just doesn't make sense. Cross-cutting is SOLID's brother, they go hand in hand.
HttpContextBase
(which was introduced for exactly this reason). I know for sure that my reality would be really sour without this class.
For logging I think it is. Logging is pervasive and generally unrelated to the service functionality. It's common and well-understood to use the logging framework singleton patterns. If you don't, you're creating and injecting loggers everywhere and you don't want that.
One issue from the above is that someone will say 'but how can I test logging?'. My thoughts are that I don't normally test logging, beyond asserting that I can actually read the log files and understand them. When I've seen logging tested, it's usually because someone needs to assert that a class has actually done something and they're using the log messages to get that feedback. I would much rather register some listener/observer on that class and assert in my tests that that gets called. You can then put your event logging within that observer.
I think caching is a completely different scenario, however.
My 2 cents ...
Yes and no.
You should never really violate the principles you adopt; but, your principles should always be nuanced and adopted in service to a higher goal. So, with a properly conditioned understanding, some apparent violations may not be actual violations of the "spirit" or "body of principles as a whole."
The SOLID principles in particular, in addition to requiring a lot of nuance, are ultimately subservient to the goal of "delivering working, maintainable software." So, adhering to any particular SOLID principle is self-defeating and self-contradictory when doing so conflicts with the goals of SOLID. And here, I often note that delivering trumps maintainability.
So, what about the the D in SOLID? Well, it contributes to increased maintainability by making your reusable module relatively agnostic to its context. And we can define the "reusable module" as "a collection of code you plan on using in another distinct context." And that applies to a single functions, classes, sets of classes, and programs.
And yes, changing logger implementations probably puts your module into a "another distinct context."
So, let me offer my two big caveats:
Firstly: Drawing the lines around the blocks of code that constitute "a reusable module" is a matter of professional judgement. And your judgement is necessarily limited to your experience.
If you don't currently plan on using a module in another context, it is probably OK for it to depend helplessly on it. The caveat to the caveat: Your plans are probably wrong -- but that's also OK. The longer you write module after module, the more and more intuitive and accurate your sense of whether "I'll need this again someday" will be. But, you will probably never be able to retrospectively say, "I've modularized and decoupled everything to the greatest extent possible, but without excess."
If you feel guilty about your errors in judgement, go to confession and move on ...
Secondly: Inverting control does not equal injecting dependencies.
This is especially true when you start injecting dependencies ad nauseam. Dependency Injection is a useful tactic for the overarching IoC strategy. But, I'd argue that DI is of lesser efficaciousness than some other tactics -- like using interfaces and adapters -- single points of exposure to the context from within the module.
And let's really focus on this for a second. Because, even if you inject a Logger
ad nauseam, you need to write code against the Logger
interface. You couldn't start using a new Logger
from a different vendor that takes parameters in a different order. That ability comes from coding, within the module, against an interface that exists within the module and which has a single submodule (Adapter) therein to manage the dependency.
And if you're coding against an Adapter, whether the Logger
is injected into that Adapter or discovered by the Adapter is generally pretty darn insignificant to the overall maintainability goal. And more importantly, if you have a module-level Adapter, it's probably just absurd to inject it into anything. It's written for the module.
tl;dr - Stop fussing about principles without consideration for why you're using the principles. And, more practically, just build an Adapter
for each module. Use your judgement when deciding where you draw the "module" boundaries. From within each module, go ahead and refer directly to the Adapter
. And sure, inject the real logger into the Adapter
-- but not into every little thing that might need it.
The idea that logging should always be implemented as a singleton is one of those lies that has been told so often it has gained traction.
For as long as modern operating systems have been about it has been recognised that you may wish to log to multiple places depending on the nature of the output.
System designers should constantly be questioning the efficacy of past solutions before blindly including them in new ones. If they're not carrying out such diligence, then they're not doing their job.
Logging genuinely is a special case.
@Telastyn writes:
Are you really never going to change your logging library, or target, or filtering, or formatting, or...?
If you anticipate that you might need to change your logging library, then you should be using a facade; i.e. SLF4J if you are in the Java world.
As for the rest, a decent logging library takes care of changing where the logging goes, what events are filtered, how log events are formatted using logger configuration files and (if necessary) custom plugin classes. There are a number of off-the-shelf alternatives.
In short, these are solved problems ... for logging ... and hence there is no need to use Dependency Injection to solve them.
The only case where DI might be beneficial (over the standard logging approaches) is if you want to subject your application's logging to unit testing. However, I suspect most developers would say that logging is not part of a classes functionality, and not something that needs to be tested.
@Telastyn writes:
And you'll then get the argument that the logger itself will handle the change. "Oh, if the target/filtering/formatting/strategy changes, then we'll just change the config!" That is garbage. Not only do you now have a God Object that handles all of these things, you're writing your code in XML (or similar) where you don't get static analysis, you don't get compile time errors, and you don't really get effective unit tests.
I'm afraid that is a very theoretical ripost. In practice, most developers and system integrators LIKE the fact that you can configure the logging via a config file. And they LIKE the fact that they aren't expected to unit test a module's logging.
Sure, if you stuff up the logging configs then you can get problems, but they will manifest as either the application failing during startup or too much / too little logging. 1) These problems are easily fixed by fixing the mistake in the config file. 2) The alternative is a complete build / analyse / test / deploy cycle each time you make a change to logging levels. That's not acceptable.
Yes and No!
Yes: I think it is reasonable that different subsystems (or semantic layers or libraries or other notions of modular bundling) each accept (the same or) potentially different logger during their initialization rather than all subsystems relying the same common shared singleton.
However,
No: it is at the same time unreasonable to parameterize logging in every little object (by constructor or instance method). To avoid unnecessary and pointless bloat, smaller entities should use the singleton logger of their enclosing context.
This is one reason among many others to think of modularity in levels: methods are bundled into classes, while classes are bundled into subsystems and/or semantic layers. These larger bundles are valuable tools of abstraction; we should give different considerations within modular boundaries than when crossing them.
First it starts with strong singleton cache, the next things you see are strong singletons for database layer introducing global state, non-descriptive APIs of class
es and untestable code.
If you decide not to have a singleton for a database, it's probably not a good idea having a singleton for a cache, after all, they represent a very similar concept, data storage, only using different mechanisms.
Using a singleton in a class turns a class having a specific amount of dependencies to a class having, theoretically, an infinite number of them, because you never know what is really hidden behind the static method.
In the past decade I have spent programming, there was only one case where I witnessed an effort to change logging logic (which was then written as singleton). So although I love dependency injections, logging is really not such a huge concern. Cache, on the other hand, I would definitely always make as a dependency.
Yes and No, but mostly No
I assume most of the conversation is based on static vs injected instance. No one is proposing that logging break the SRP I assume? We're mainly talking about the "Dependency inversion principle". Tbh I mostly agree with Telastyn's no answer.
When is it okay to use statics? Because clearly it is sometimes okay. The yes answers point of the benifits of abstraction and the "no" answers point out that they're something you pay for. One of the reasons your job is hard is there isn't one answer you can write down and apply to all situations.
Take:
Convert.ToInt32("1")
I prefer this to:
private readonly IConverter _converter;
public MyClass(IConverter converter)
{
Guard.NotNull(converter)
_converter = conveter
}
....
var foo = _converter.ToInt32("1");
Why? I'm accepting that I will need to refactor the code if I need the flexibility to swap out conversion code. I'm accepting that that I won't be able to mock this out. I believe that the simplicity and terseness is worth this trade.
Looking at the other end of spectrum, if IConverter
was a SqlConnection
, I would be fairly horrified to see that as a static call. The reasons why are faily obvious. I'd point out that a SQLConnection
can be fairly "cross-cutting" in an applciation, so I wouldn't have used those exact words.
Is Logging more like a SQLConnection
or Convert.ToInt32
? I'd say more like 'SQLConnection`.
You should be mocking Logging. It talks to the outside world. When writing a method using Convert.ToIn32
, I'm using it as a tool to calculate some other seperately testable output of the class. I don't need to check Convert
was called correctly when checking that "1" + "2" == "3". Logging is different, it's a entirely indepedent output of the class. I'm assuming it's an output that has value to you, the support team and the business. Your class isn't working if the logging isn't right, so the unit tests shouldn't pass. You should be testing what your class logs. I think this is the killer argument, I could really just stop here.
I also think it is something that's quite likely to change. Good logging doesn't just print strings, it's a view into what your application is doing (I'm a big fan of event based logging). I've seen basic logging turn into quite elaborate reporting UIs. It obviously a lot easier to head in this direction if your logging looks like _logger.Log(new ApplicationStartingEvent())
and less like Logger.Log("Application has started")
. One might argue that that this is creating inventory for a future that may never happen, this is a judgement call and I happen to think it's worth it.
In fact in a personal project of mine, I created a non logging UI purely using the _logger
to figure out what the application was doing. This meant I didn't have to write code to figure out what the application was doing, and I ended up with rock solid logging. I feel like if my attitude to logging was that it's simple and unchanging, that idea wouldn't have occured to me.
So I'd agree with Telastyn for the case of logging.
Semantic Logging Application Block
. Don't use it, like most of the code created by the MS "patterns and pratice" team, ironically, it's an antipattern.
May 14, 2016 at 17:47
First Cross cutting concerns are not major building blocks and shouldn't be treated as dependencies in a system. A system should work if e.g. Logger is not initialized or cache is not working. How will you make system less coupled and cohesive? That's where SOLID comes into picture in OO system design.
Keeping object as singleton has nothing to do with SOLID. That's your object life cycle how long you want the object to live in memory.
A class that needs an dependency to initialize shouldn't know if the class instance supplied is singleton or transient. But tldr; if you are writing Logger.Instance.Log() in every method or class then it's problematic code (code smell/hard coupling) a really messy one. This is the moment when people start abusing SOLID. And fellow developers like OP start asking genuine question like this.
I've solved this problem using a combination of inheritance and traits (also called mixins in some languages). Traits are super handy for solving this kind of cross cutting concern. It's usually a language feature though so I think the real answer is that it's dependent on language features.