12

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.

closed as primarily opinion-based by user40980, BЈовић, Ross Patterson, GlenH7, gnat Mar 11 '15 at 4:46

Many good questions generate some degree of opinion based on expert experience, but answers to this question will tend to be almost entirely based on opinions, rather than facts, references, or specific expertise. If this question can be reworded to fit the rules in the help center, please edit the question.

31

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.

  • 11
    +! keep logger out of constructor parms; i prefer a static logger so i know everyone is using the same thing – Steven A. Lowe Mar 9 '15 at 21:01
  • 1
    @StevenA.Lowe Note though that using static loggers may lead to other problems down the line if you are not careful (eg. static initialization order fiasco in C++). I agree that having the logger as a globally accessible entity has its charms, but it should be carefully evaluated whether and how such a design fits into the overall architecture. – ComicSansMS Mar 10 '15 at 12:22
  • @ComicSansMS: of course, plus threading issues etc. Just a personal preference - "as simple as possible, but not simpler" ;) – Steven A. Lowe Mar 10 '15 at 21:54
  • Having a static logger can be a problem. It makes dependency injection harder (unless you mean a singleton logger instantiated by your DI container), and if you ever want to change your architecture it may be painful. Right now, for instance, I'm using Azure functions which pass in a logger as a parameter to each function execution, so I regret passing my logger through the constructor. – Slothario Jan 18 at 18:16
  • @Slothario: That's the beauty of a static logger. No injection of any kind is required. – Robert Harvey Jan 18 at 18:22
7

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.

3

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?).

  • It seems awkward to me to use property injection to call calculateFooBarSum. – an phu Jun 26 '17 at 23:31
2

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)

1

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.

  • 1
    Of course this has the drawback that the logger is unavailable to the constructor. – Ben Voigt Mar 10 '15 at 0:20
  • 1
    I really like this answer. It makes logging a side-concern of the class that you have to care about separately from just using it. Chances are that if you're adding the logger to the constructor of a large number of classes, you're probably using dependency injection. I can't speak for all languages, but I know in C#, some DI implementations will also inject directly to properties (getter/setter). – jpmc26 Mar 10 '15 at 6:36
  • @BenVoigt: That is true, and it may be a killer reason not to do it this way but, usually, you can do the logging you'd otherwise do in the constructor in response to a logger being set. – Jack Aidley Mar 10 '15 at 8:37
0

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).

  • 1
    hmmm... so you want your class to perform logging (logging should be in the specification) but you don't want to test using a logger. Is this possible? I think your point is a non-go. Clearly if your testing tools are broken you can't test -- to design in a way to not rely on a testing tool is a bit of over-engineering IMHO – Hogan Mar 10 '15 at 13:42
  • 1
    My point is that if I use a constructor and call a method on a class and it still fails because I didn't set a property, then the designer of the class has misunderstood the concept behind a constructor. If a logger is required by the class, it should be injected in the constructor - that is what the constructor is there for. – Dan Pantry Mar 10 '15 at 13:55
  • umm.. no not really. If you consider the Logging system part of the "framework" then it does no make sense as part of the constructor. But this has been stated in other answers clearly. – Hogan Mar 10 '15 at 13:59
  • 1
    I am arguing against property injection. I am not necessarily advocating for the use of injecting it in the constructor. I am just saying that in my opinion, that is preferable to property injection. – Dan Pantry Mar 10 '15 at 14:01
  • "Is this possible?" also, yes, IL weaving is a thing that exists and was mentioned in the top answer... mono-project.com/docs/tools+libraries/libraries/Mono.Cecil – Dan Pantry Mar 10 '15 at 14:06
0

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.

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