43

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?

  • 27
    I've found that the SOLID principles are mostly only useful for those who already have the experience necessary to unknowingly follow them anyway. – svidgen May 13 '16 at 18:43
  • 1
    I've found the term "cross-cutting concern" to be a pretty specific term (tied to aspects) and to not necessarily be synonymous with singleton. Logging can be a cross cutting concern in the sense that you sometimes want to log a generic message in multiple places in the same way (e.g. log every call to a service method with who made the call or some other audit type logging). However, I would argue that logging (in the general sense) is not a cross cutting concern just because it is used everywhere. I think this is more of a "ubiquitous concern" although that isn't really a standard term. – Pace May 13 '16 at 21:46
  • 4
    @svidgen the SOLID principles are also useful when those who don't know them review your code and ask you why you did it that way. It's nice to be able to point at a principle and say, "that's why" – candied_orange May 14 '16 at 2:02
  • 1
    Do you have any examples why do you think logging is a special case? It's quite simplistic channel to redirect some data to, usually part of whatever X framework you are using. – ksiimson May 14 '16 at 15:20

11 Answers 11

42

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.

  • 22
    What alternative do you have then? Injecting an ILogger into every class you write? Writing a decorator for every class that needs a logger? – Robert Harvey May 13 '16 at 14:15
  • 16
    Yeah. If something is a dependency make it a dependency. And if that means too many dependencies, or you're passing a logger somewhere it shouldn't go - good, now you can fix your design. – Telastyn May 13 '16 at 14:27
  • 7
    @RobertHarvey en.wikipedia.org/wiki/Aspect-oriented_programming – JAB May 13 '16 at 15:35
  • 20
    The real issue I have with dogmatic IoC is that pretty much all modern code depends on something that is neither injected nor abstracted away. If you write C#, for instance, you're not often going to abstract 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. – svidgen May 13 '16 at 18:40
  • 6
    @svidgen - I hope you didn't read my answer as advocating dogmatic IoC. And just because we don't abstract away these core types (and other things where prudent) doesn't mean it's okay to have a global List/String/Int/etc. – Telastyn May 13 '16 at 19:16
39

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.

  • 9
    A reasoned, practical, non-dogmatic answer. – user1936 May 13 '16 at 16:49
  • 11
    Can you please tell us why all the five SOLID principles get broken by cross-cutting concerns? I have problems to understand that. – Doc Brown May 13 '16 at 18:44
  • 4
    Yeah. I could potentially think of a good reason to "break" each principle individually; but not a single reason to break all 5! – svidgen May 13 '16 at 19:24
  • 1
    It would be ideal for me not to have to bang my head against the wall every time I need to mock a singleton logger/cache for a unit test. Imagine what it would be like to unit test a web application without having HttpContextBase (which was introduced for exactly this reason). I know for sure that my reality would be really sour without this class. – devnull May 13 '16 at 19:42
  • 2
    @devnull maybe that's the issue then, rather than the cross cutting concerns themselves... – Boris the Spider May 14 '16 at 8:25
25

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.

  • 3
    Having dabbled a bit in FP by now I've come to grow a large interest in using (maybe monadic) function composition to embed stuff like logging, error handling etc in your use-cases without putting any of it in the core business logic (see fsharpforfunandprofit.com/rop) – sara May 13 '16 at 13:12
  • 5
    Logging shouldn't be pervasive. If it is, then you're logging far too much and you're just creating noise for yourself when you actually need to go look at those logs. By injecting the logger dependency, you're forced to think about whether or not you actually need to log something. – RubberDuck May 13 '16 at 17:43
  • 12
    @RubberDuck - Tell me that "logging shouldn't be pervasive" when I get a bug report from the field and the only debug ability I have to figure out what happened is that log file that I didn't want to make pervasive. Lesson learned, logging should be "pervasive", very pervasive. – Dunk May 13 '16 at 18:29
  • 15
    @RubberDuck: With server software, logging is the only way to survive. It's the only way to figure out a bug that happened 12 hours ago instead of right now on your own laptop. Don't worry about noise. Use log management software so you can query your log (ideally you should also set up alarms that will email you on error logs) – slebetman May 13 '16 at 19:16
  • 6
    Our support staff call me and tell me "the customer was clicking some pages and an error popped up." I ask them what the error said, they don't know. I ask what specifically the customer clicked, they don't know. I ask if they can reproduce, they cannot. I agree logging can mostly be achieved with well deployed loggers in a few key hinge points but the little odd messages here and there are invaluable as well. Fear of logging leads to lower quality software. If you end up with too much data, prune back. Don't optimize prematurely. – Pace May 15 '16 at 1:18
12

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.

  • 4
    Man ... I would've given a shorter answer, but I didn't have the time. – svidgen May 13 '16 at 18:30
  • 2
    +1 for using an Adapter. You need to isolate dependencies on 3rd-party components in order to allow them to be replaced wherever possible. Logging is an easy target for this -- many logging implementations exist and mostly have similar APIs, so a simple adapter implementation can allow you to change them very easily. And to people who say you'll never change your logging provider: I have had to do this, and it caused a real pain because I wasn't using an adapter. – Jules May 16 '16 at 15:17
  • "The SOLID principles in particular, in addition to requiring a lot of nuance, are ultimately subservient to the goal of "delivering working, maintainable software."" - This is one of the best statements I have seen. As a mildly-OCD coder, I have had my share of struggles with idealism vs. productivity. – DVK Dec 8 '16 at 19:56
  • Every time I see a +1 or a comment on an old answer, I read my answer again and discover, again, that I'm a terribly disorganized and unclear writer... I appreciate the comment though, @DVK. – svidgen Dec 8 '16 at 20:13
  • Adapters are life savers, they don't cost much and make your life a lot easier, specially when you do want to apply SOLID. Testing is a breeze with them. – Alan Aug 23 '18 at 14:24
8

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.

  • 2
    What is your proposed solution, then? Is it injecting an ILogger into every class you write? What about using a Decorator pattern? – Robert Harvey May 13 '16 at 14:13
  • 4
    Not really answering my question now, is it? I stated the patterns merely as examples... It doesn't have to be those if you have something better in mind. – Robert Harvey May 13 '16 at 14:16
  • 8
    I don't really understand this answer, in terms of any concrete proposals – Brian Agnew May 13 '16 at 14:44
  • 3
    @RobbieDee - So your are saying that Logging should be implemented in the most convoluted and inconvenient way by the "off-chance" that we might want to log to multiple places? Even if a change like that occurred, do you really think adding that functionality to the existing Logger instance is going to be more work than all the effort to pass that Logger around between classes and changing interfaces when you decide you want a Logger or not on the dozens of projects where that multiple place logging will never occur? – Dunk May 13 '16 at 18:39
  • 2
    Re: "you may wish to log to multiple places depending on the nature of the output": Sure, but the best way to handle that is in the logging framework, rather than trying to inject multiple separate logging dependencies. (The common logging frameworks already support this.) – ruakh May 14 '16 at 19:35
4

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.

3

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.

3

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 classes 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, logging rarely changes and usually does not have to be a swappable module. However, I once tried to unit-test a logging helper class, which had a static dependency on the logging system. I decided the easiest mechanism to make it testable would be to run the class under test in a separate process, configure its logger to write to STDOUT, and parse that output in my test case ಠ_ಠ I've had similar experience with clocks where you'd obviously never want anything but the real time, right? Except when testing time zone/DST edge cases, of course… – amon May 14 '16 at 16:03
  • @amon: Clocks are like logging in that there's already another mechanism that serves the same purpose as DI, namely Joda-Time and its many ports. (Not that there's anything wrong with using DI instead; but it's simpler to use Joda-Time directly than to try to write a custom injectible adapter, and I've never seen anyone regret doing so.) – ruakh May 16 '16 at 13:55
  • @amon "I've had similar experience with clocks where you'd obviously never want anything but the real time, right? Except when testing time zone/DST edge cases, of course…" -- or when you realise that a bug has corrupted your database and the only hope of getting it back is to parse your event log and replay it starting from the last backup... but suddenly you need all your code to work based on the timestamp of the current log entry rather than the current time. – Jules May 16 '16 at 15:21
3

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.

  • This is how I log incidently. I'd like to link to an article or something, but I can't find one. If you are in the .NET world, and look up event logging, you'll find 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. – Nathan Cooper May 14 '16 at 17:47
3

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.

2

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.

  • Interesting. I've never done any significant work using a language that supports traits, but I've considered using such languages for some future projects, so it would be helpful to me if you can expand on this, and show how traits solve the problem. – Jules May 16 '16 at 15:27

protected by gnat May 15 '16 at 14:29

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.