44

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?

Den
  • 4,827
  • 2
  • 32
  • 48
  • 28
    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 02: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 Answers11

43

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.

Klom Dark
  • 588
  • 3
  • 5
  • 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
  • 3
    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
  • @devnull Even if the logger was injected with classic dependency injection, if it was used everywhere, you would still need to mock it out in every test case you have. – Pace May 13 '16 at 21:39
  • @Pace at least I would be able to easily mock it – devnull May 14 '16 at 08:20
  • 1
    @devnull logging shouldn't really be part of unit testing - it's not part of the logic under test. If it is, then it should certainly be SOLID. If it isn't, then most logging frameworks support a specific test configuration, that usually turns logging off; unless you're debugging tests. – Boris the Spider May 14 '16 at 08:21
  • 1
    @BoristheSpider the singleton logger is part of the method under test. in order for that method to work I would need the singleton instance to begin with and all the setup required in order for it to work. – devnull May 14 '16 at 08:23
  • @devnull No sensible logging framework uses a singleton logger - it uses a factory pattern. And as I said, logging frameworks worth their salt will support a test configuration of some sort, where you can start up the logging framework and forget about it. – Boris the Spider May 14 '16 at 08:24
  • 1
    @BoristheSpider the question is about singleton logger/cache. and configuring a logger would make it an integration test since now it depends on a configuration store of some sort (web/app.config, db or what not). – devnull May 14 '16 at 08:25
  • 2
    @devnull maybe that's the issue then, rather than the cross cutting concerns themselves... – Boris the Spider May 14 '16 at 08:25
  • 1
    @devnull I mock all logging with one call, `LogUtil.mock()` which installs a logger that dumps all to standard out. I don't consider a test an "integration" test because it depends on logging the same way I don't consider a test an integration test because it depends on the static Math library. Follow the spirit of the law, not the letter, but yes, question when the two diverge. – Pace May 15 '16 at 01:13
  • 1
    SRP is the main SOLID part that has trouble with cross-cutting things. Logging is a key example. You can get around this with the decorator pattern, mind you, but sometimes it's tricky and introduces more complexity and difficulty to try to stay SOLID than it does to just allow that one exception - and in the end, simple code that's easy to work with is the aim of SOLID anyway. – anaximander May 15 '16 at 16:55
  • 1
    @BoristheSpider> I have seen logging output as part of customer requirements quite a few times. Arguing you do not want to test that looks very fishy to me. And even when not part of the requirements, when something escalates all the way to my desk, I'd rather be confident the logs I get are correct. So… yes, we assert that some operations generate logs and we check the content is correct. – spectras May 07 '20 at 13:48
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.

Telastyn
  • 108,850
  • 29
  • 239
  • 365
  • 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
  • 17
    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 https://en.wikipedia.org/wiki/Aspect-oriented_programming – JAB May 13 '16 at 15:35
  • Having components retrieve a logger from a central location rather than injecting it into *every* class in the system doesn't preclude you from mocking things. – 17 of 26 May 13 '16 at 17:36
  • @JAB: Pretty sure that things like code weaving and method interception are not part of proper SOLID. – Robert Harvey May 13 '16 at 17:46
  • 1
    @RobertHarvey SOLID applies to your source code, not to what it compiles to. Most object-oriented code gets compiled to the same sort of instructions old-school procedural code does. – JAB May 13 '16 at 17:58
  • 1
    @17of26 - absolutely it does. Or rather you can mock it the _same way_ for all of your tests, or you can _not_ run your tests in parallel. These are both deal breakers for me, and most others. – Telastyn May 13 '16 at 17:58
  • @JAB: Eh, not sure how that is relevant. Old-school OO doesn't make use of AOP; there's always some workaround, and it doesn't always occur at the compiled level. – Robert Harvey May 13 '16 at 18:00
  • @RobertHarvey That was my point, the details of how AOP is implemented are irrelevant if you use existing tools for it, much as the details of how compilers go from source to executables are irrelevant when following the principles of SOLID. Just because OO languages don't traditionally utilize AOP doesn't mean they can't, it's simply an alternative to whatever other workarounds you may have. – JAB May 13 '16 at 18:04
  • @JAB The source code for AOP looks very different from classical OOP, and it wouldn't surprise me if most AOP implementations violate SOLID in some way. – Robert Harvey May 13 '16 at 18:05
  • @Telastyn Fair point, I had not considered that restriction. – 17 of 26 May 13 '16 at 18:07
  • 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
  • 4
    @svidgen is exactly right, see [Enterprise FizzBuzz](https://github.com/EnterpriseQualityCoding/FizzBuzzEnterpriseEdition). And saying "the logger _itself_ will handle the change" is definitely not garbage. Using NLog directly has never let me down; any silly intermediate layer I might have added is much more likely to hurt than help. – default.kramer May 13 '16 at 19:16
  • 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
  • 2
    @Telastyn Your answer is "no" ... so, I kind of *do* read it that way! – svidgen May 13 '16 at 19:20
  • @svidgen - edited a bit to clarify. – Telastyn May 13 '16 at 19:25
  • @Telastyn Good edit! I don't fully agree -- but it's more clear and much more reasonable sounding now! – svidgen May 13 '16 at 19:30
  • 1
    why would you have all your classes depend on a logger? I haven't really seen any use for logging every method call in every object of every class, but maybe I'm in the wrong domain. – sara May 13 '16 at 20:43
  • 2
    @kai - _I_ don't. But the question indicates it's used everywhere, and about half the codebases I see use logging as a sort of printf debugging, with the argument being that they can't reproduce things without real load/environments. IMO, passing the logger around (or similar explicit dependencies) helps push back against that idea. – Telastyn May 13 '16 at 20:46
  • 1
    @default.kramer How would you write unit tests that check that the component under test writes a specific log entry with NLog directly? Logging should definitely be tested and not just haphazardly thrown on top of existing code (I've seen components that log the same error five times..). – Voo May 15 '16 at 14:33
  • @default.kramer - "Using NLog directly has never let me down; any silly intermediate layer I might have added is much more likely to hurt than help" ... I've never used NLog, as my .net experience is more oriented towards desktop apps, but in Java I have been bitten before by using a logging system directly and finding I needed to change it (due to a particularly nasty bug, in this case). I would never again rely on a third party library and scatter dependencies to it throughout my code, because it took me days to clean up the mess last time. – Jules May 16 '16 at 15:03
  • @RobertHarvey - "What alternative do you have then? Injecting an ILogger into every class you write?" ... Yes. Injecting a logger is 3 lines of code, if you use a DI framework with autowiring support. It's also 3 lines of code if you're handing-building your objects, but those 3 lines are scattered all over the place. It's not hard, it gives you flexibility to change your logging implementation more easily, which *you may find you need to do some day*, so why not do it? – Jules May 16 '16 at 15:09
  • 2
    @jules: `Trace()` works perfectly fine for me. It's a static method in the `System.Diagnostics` namespace. Just sayin'. Also YAGNI. I'm not saying your approach is wrong; I'm saying that programmers have a propensity for complicating things without properly evaluating the costs. – Robert Harvey May 16 '16 at 15:12
27

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.

Brian Agnew
  • 4,676
  • 24
  • 19
  • "you're creating and injecting loggers everywhere and you don't want that" - creation and injection can be handled by IoC, so not a real problem in my opinion. I have a bigger problem with logging polluting the actual business logic, which is unavoidable. – Den May 13 '16 at 12:53
  • 5
    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 https://fsharpforfunandprofit.com/rop/) – sara May 13 '16 at 13:12
  • "creation and injection can be handled by IoC" - again, IoC is a pattern. You may have a container that does this automatically (via reflection or similar), but IoC doesn't mandate such a mechanism – Brian Agnew May 13 '16 at 14:11
  • 1
    @kai: Indeed, logging is just the Writer Monad. And Configuration, another thing that is often designed as a global singleton, is the Reader Monad. (And, contrary to popular belief, monads are a way to structure programs, *any* programs, also imperative and object-oriented ones, not just some deep functional programming magic.) – Jörg W Mittag May 13 '16 at 16:05
  • 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.](http://thecodelesscode.com/case/103?lang=pt&topic=logging) 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
  • 14
    @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
  • 18
    @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
  • @slebetman do you know what happens when you log *everything* **and** send email alerts? *People filter them directly into their trash.* Don't get me wrong, logs and email alerts are *absolutely invaluable*, but only IFF you're logging/alerting to the right things. – RubberDuck May 14 '16 at 14:30
  • 2
    @RubberDuck: Only if you allow them to. My previous company don't. We prioritised fixing server issues over almost everything else. They're considered show-stopper bugs that must be fixed or explained within 24 hours. If they can be explained away as normal operations then we disable the alerts and keep an eye on the stats. But without logging EVERYTHING we can't possibly monitor warning and performance graphs. Like I said. Don't use a text editor to monitor your log. Use a log parsing tool like loggly or kibana. – slebetman May 14 '16 at 20:19
  • 1
    @RubberDuck - "log everything and send email alerts" and why would anyone do that ? – Brian Agnew May 14 '16 at 21:38
  • I've no idea @BrianAgnew. That's my point. If you inject your logger as a dependency, you stop to think about whether or not you're actually logging at the correct level of abstraction. IMO, only your controllers, presenters, etc need to be doing any logging. Not every little class in the world needs to have a logger instance, that's why they invented the stack trace... – RubberDuck May 14 '16 at 21:42
  • 7
    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 01:18
  • Also, we have a soak test that runs weekly, it extrapolates when our 40MB limited logs would run out. Our trace log must last 24 hours (for in house test bugs), our debug log 3 days (worst case time to get logs from customers), all other logs must last 30 days (for sporadic issues our customers don't mention until we call and for auditing) – Pace May 15 '16 at 01:32
  • 1
    "My thoughts are that I don't normally test logging". I've seen code bases where logging was just haphazardly introduced wherever someone thought they needed it (no concept where to log what and when). Half the time, some things don't get logged at all and the other time a single error produces five identical log entries. Logging is important but like everything you should have a concept for it. And if you do, why shouldn't you test it like every other functional part of your application? – Voo May 15 '16 at 14:37
15

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.

svidgen
  • 13,414
  • 2
  • 34
  • 60
  • 5
    Man ... I would've given a shorter answer, but I didn't have the time. – svidgen May 13 '16 at 18:30
  • 3
    +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 08 '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 08 '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.

Robbie Dee
  • 9,717
  • 2
  • 23
  • 53
  • 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
  • 1
    A bit wild, but I'm going to go with thinking about the design rather than [cargo cult programming](https://en.wikipedia.org/wiki/Cargo_cult_programming)... 8-D – Robbie Dee May 13 '16 at 14:15
  • 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
  • Some frameworks have included logging out of the box, some developments have used log4net, some have rolled their own (non-singleton) ILogger implementations, some have required MQ. As to what we'd use next - who knows... – Robbie Dee May 13 '16 at 14:32
  • 8
    I don't really understand this answer, in terms of any concrete proposals – Brian Agnew May 13 '16 at 14:44
  • @BrianAgnew Yep, that's kind of the point. If you want egg on your face, simply say that this is the way things should *always* be done. You'll be proved wrong time and time again. Things move on. – Robbie Dee May 13 '16 at 16:18
  • At the same time (and I don't think the question is asserting that this should be the only approach) you need some direction in terms of what's acceptable *right now* – Brian Agnew May 13 '16 at 16:54
  • That ship has already sailed. Otherwise bright people get that shot-gunning singletons everywhere is a bad idea but desperately cling onto the logging dogma. It is the twitching corpse of a dead idea and it is time to let it go... – Robbie Dee May 13 '16 at 17:34
  • 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
  • @Dunk Look at the problem - analyse, think! If what you've used before fits - great. But understand *why* and be able to justify the design decisions. – Robbie Dee May 13 '16 at 18:59
  • 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
  • @ruakh Yep, that's exactly what we did when we rolled our own. – Robbie Dee May 16 '16 at 07:51
  • "System designers should constantly be questioning the efficacy of past solutions before blindly including them in new ones" -> I can twist your argument here and say that "SOLID must never be broken" is one of those lies that has been told so often it has gained traction. – T. Sar May 16 '16 at 20:37
  • @ThalesPereira Then you'd be wasting your time - I didn't mention SOLID once... – Robbie Dee May 16 '16 at 20:45
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.

Stephen C
  • 25,180
  • 6
  • 64
  • 87
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.

Erik Eidt
  • 33,282
  • 5
  • 57
  • 91
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.

Andy
  • 10,238
  • 4
  • 25
  • 50
  • 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.

Nathan Cooper
  • 1,319
  • 9
  • 15
  • [This is how I log incidently](https://github.com/NathanLBCooper/ProcessGremlin/tree/master/Logging). 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.

vendettamit
  • 151
  • 3
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.

RibaldEddie
  • 3,168
  • 1
  • 15
  • 17
  • 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