In my code I inject a logger to many of my classes through their constructor’s parameter list
I noticed that I put it randomly: sometimes it’s the first on the list, sometimes last, and sometimes in between
Do you have any preference? My intuition says that consistency is helpful in this case and my personal preference is to put it first so it will be easier to be noticed when it’s missing and easier to skip when it’s there.
Loggers are what we call a “cross-cutting concern.” They yield to techniques such as Aspect-Oriented Programming; if you have a way to decorate your classes with an attribute or perform some code weaving, then that is a good way to get logging capabilities while keeping your objects and parameter lists “pure.”
The only reason you might want to pass in a logger is if you wanted to specify different logging implementations, but most logging frameworks have the flexibility to allow you to configure them, say, for different logging targets. (log file, Windows Event Manager, etc.)
For these reasons, I prefer to make logging a natural part of the system, rather than passing a logger into every class for logging purposes. So what I generally do is reference the appropriate logging namespace, and simply use the logger in my classes.
If you still want to pass in a logger, my preference is that you make it the last parameter in the parameter list (make it an optional parameter, if possible). Having it be the first parameter doesn’t make much sense; the first parameter should be the most important one, the one most relevant to the class’s operation.
5
In languages with function overloading, I’d argue that the more likely an argument is to be optional, the further right it should be. This creates consistency when you create overloads where they’re missing:
foo(mandatory);
foo(mandatory, optional);
foo(mandatory, optional, evenMoreOptional);
In functional languages the reverse is more useful – the more likely you are to choose some default, the further left it should be. This makes it easier to specialize the function by simply applying arguments to it:
addThreeToList = map (+3)
However as mentioned in the other answers, you probably don’t want to explicitly pass the logger in the argument list of every class in the system.
You can definitely spend a lot of time over-engineering this problem.
For languages with canonical logging implementations, just instantiate the canonical logger directly in every class.
For languages without a canonical implementation, try to find a logging facade framework and stick to it. slf4j is a good choice in Java.
Personally I’d rather stick to a single concrete logging implementation and send everything to syslog. All the good log analysis tools are capable of combining sysout logs from multiple app servers into a comprehensive report.
When a function signature includes one or two dependency services as well as some “real” arguments, I place the dependencies last:
int calculateFooBarSum(int foo, int bar, IntegerSummationService svc)
Since my systems tend to only have five or fewer such services, I always make sure the services are included in the same order across all function signatures. Alphabetical order is as good as any. (Aside: maintaining this methodological approach for mutex handling will also reduce your chances of developing deadlocks.)
If you find yourself injecting more than a dozen or so dependencies across your app, then the system probably needs to be split up into separate subsystems (dare I say microservices?).
1
Loggers are a bit of a special case because they have to be available literally everywhere.
If you’ve decided that you want to pass a logger into every class’ constructor, then you should definitely set a consistent convention for how you do that (eg, always the first parameter, always passed by reference, the constructor initialization list always starts with m_logger(theLogger), etc). Anything that’s going to be used throughout your entire codebase is going to benefit from consistency someday.
Alternatively, you could have every class instantiate their own logger object, without needing anything to be passed in. The logger may need to know a few things “by magic” for that to work, but hardcoding a filepath in the class definition is potentially a lot more maintainable and less tedious than passing it correctly to hundreds of different classes, and arguably a lot less evil than using a global variable to bypass said tedium. (Admittedly, loggers are among the very few legitimate use cases for global variables)
I agree with those suggesting that the logger should be statically accessed rather than passed into classes. However if there is a strong reason you want to pass it in (perhaps different instances want to log to different locations or something) then I would suggest you do not pass it using the constructor but rather make a separate call to do so, e.g. Class* C = new C(); C->SetLogger(logger);
rather than Class* C = new C(logger);
The reason for preferring this method is that the logger is not an essentially part of the class but rather an injected feature used for some other purpose. Placing it in the constructor list makes it a requirement of the class and implies it is part of the actual logical state of the class. It is reasonable to expect, for example (with most classes although not all), that if X != Y
then C(X) != C(Y)
but it is unlikely that you would test logger inequality if you are comparing too instances of the same class.
3
It’s worth mentioning, something I have not seen the other answers touch on here, is that by making the logger injected via property or static, it makes it hard(er) to unit test the class. For example, if you make your logger injected via property, you will now have to inject that logger every time you test a method that uses the logger. This means you might as well have it sets as a constructor dependency because the class does require it.
Static lends itself to the same issue; if the logger doesn’t work, then your entire class fails (if your class uses the logger) – even though the logger is not necessarily ‘part’ of the responsibility of the class – although it’s not nearly as bad as property injection because you at least know that the logger is always “there” in a sense.
Just some food for thought, especially if you employ TDD. In my opinion, a logger should really not be part of a testable part of a class (when you test the class, you shouldn’t be testing your logging as well).
5
I’m too lazy to pass around a logger object to each class instance. So, in my code, these kinds of things either sit in a static field or a thread-local variable in a static field. The latter is kind of cool and lets you use a different logger for each thread and lets you add methods to turn logging on and off that do something meaningful and expected in a multi-threaded application.