1

I see often classic constructs like this:

if (LOG.isLoggable(Level.FINER)) {
  LOG.finer("The driver of the car is '"+ car.getDriver().getName() +"'.");
}

Assuming the car has no driver, the getDriver() returns null.

The implementation of LOG.finer can be found here: http://developer.classpath.org/doc/java/util/logging/Logger-source.html#line.971

Then:

  1. getName() can not be executed on null so an NPE will be thrown. Only under one special circumstance: I must have the logger on FINER.
  2. LOG.isLoggable must be executed twice, 1st before the method-call of .finer( and inside the method finer a second time.
  3. A StringBuilder is created on the mathematical operatior +.
  4. I must import the class Level.
  5. A different Thread could set the Driver to null and prevent the log from logging this line.

What if I use Lambdas instead?

Example:

LOG.finer(()->"The driver of the car is '", ()->car.getDriver().getName(), ()->"'.");

Wherat finer is defined as

public void finer(ObjectReturningFunctionalInterface ... arguments) {

We can solve all the cons we see in the classic style if we catch all argument evaluation exceptions.

Why is it a bad idea?

Grim
  • 1,938
  • 10
  • 56
  • 123
  • What is the `LOG.finer` method actually performing as an operation? Your question is not clear on why do you think is it a bad idea? There would still be an NPE in the second approach. And the part inside the *Then* section just kind of not relate to the question either. Could you share some code to elaborate what you mean? – Naman Feb 26 '19 at 02:29
  • @nullpointer "We can solve all the cons we see in the classic style if we catch all argument evaluation exceptions." [wording fixed] – John Kugelman Feb 26 '19 at 02:32
  • 1
    A better idea I think, a single lambda which generates the whole string. In this sense anything that may potentially have a cost to create (rare, imo) will only be generated if the level is used. Using varargs lambdas for a single string just seems excessive – Rogue Feb 26 '19 at 03:07
  • @Rogue Thats what I thought too, but case 5 wont be solved then. And you are right, your point is a good answer. – Grim Feb 26 '19 at 07:50
  • Close-as: **primarily opinion-based** is described as `tend to be almost entirely based on opinions, rather than facts, references, or specific expertise.`. 1-5 are hard facts. – Grim Feb 26 '19 at 07:53
  • 2
    @PeterRader Those may be facts but the question title will likely attract opinions. And are the cons you listed even that bad? In my opinion you should stay with the classic way over having 3 lambdas just for clarity. Even checking the log level before logging isn't something I would do in this case. Most logger frameworks already allow you to add lambdas to provide arguments btw. Edit: the new title doesn't change much.. "better" is stil subjective unless you clarify what exactly you want to achieve (e.g. performance, readability, avoiding nullpointers) – kapex Feb 26 '19 at 08:03
  • There are probably two deeper problems here: If you find yourself running into NPE, avoid using `null`. If you find JUL lacking, use a better logging framework (SLF4J solves 2,3,4). – Thilo Feb 26 '19 at 08:04
  • 1
    3 is untrue. 1 is a problem either way. 2 is because your logging idiom is from about 5 years ago and literally noone writes logging like this. The last two points seem spurious at best. TL;DR: use a decent interpolated logging famework. – Boris the Spider Feb 26 '19 at 08:06
  • @kapex Checking the logging level before logging is made because of performance reasons. If we dont check the logging-level the Point 3 (+ operator) will be a perfomance bottleneck in production environments. – Grim Feb 26 '19 at 08:07
  • 1
    Also [`logger.finer()`](https://docs.oracle.com/en/java/javase/11/docs/api/java.logging/java/util/logging/Logger.html#fine(java.util.function.Supplier)) already exists!! Your suggestion of multiple lambdas adds literally nothing. – Boris the Spider Feb 26 '19 at 08:08
  • 2
    "Checking the logging level before logging is made because of performance reasons" Because you use a terrible logging framework. The modern ones interpolate into String constants. No overhead (i.e. String concat, `toString` calls on parameters) in that. – Thilo Feb 26 '19 at 08:08
  • @BoristheSpider Good catch! Multiple lambdas solve point 5 - one supplier wont solve point 5. – Grim Feb 26 '19 at 08:09
  • Multiple lambdas does not seem to solve Point 5. Immutable data structures do. – Thilo Feb 26 '19 at 08:13
  • @Thilo Catching NPE in every lambdas not prevent logging the line! See my line `(...) if we catch all argument evaluation exceptions`. – Grim Feb 26 '19 at 08:15
  • I thought 5 was about different threads changing the data to something that should not be logged (i.e. wrong name), and 1 was about exceptions. Anyway, give SLF4J a spin (it does not support lambdas yet, though: https://jira.qos.ch/browse/SLF4J-371). https://stackoverflow.com/questions/10555409/logger-slf4j-advantages-of-formatting-with-instead-of-string-concatenation – Thilo Feb 26 '19 at 08:19
  • Have you actually measured performance? Any performance gains by avoiding string concatenation or checking log levels twice is neglectable compared to what you could gain by switching to other log frameworks. If "getDriver" is a costly operation then why not use one of the existing methods that allow you to supply parameters or messages with lambdas? – kapex Feb 26 '19 at 08:23
  • @Thilo SLF4J have legal restrictions that makes additional work required. I really love SLF4J btw. – Grim Feb 26 '19 at 08:24
  • @kapex Performance is a good point, see Rogue's comment, its pretty the same as your argument. – Grim Feb 26 '19 at 08:26
  • If you have license issues with SFL4J I guess your best option is to make a Log helper static method that produces the Supplier for `logger.finer` using `String.format`. That helper can then catch all errors in the arguments. – Thilo Feb 26 '19 at 08:28
  • Sorry, in my last comment I wanted to say: If "getDriver" is a costly operation then why not implement it like similar methods of other frameworks, that allow you to supply parameters or messages with lambdas? Having interpolated strings with parameter suppliers will be way more readable. – kapex Feb 26 '19 at 08:31
  • @kapex I get your point and got it from your previous comment. Even if I hope `getDriver` is no costly operation it technically can be! Actually all messages (2) and parameters (1) are provided as seperate lambdas in the suggested rework. – Grim Feb 26 '19 at 08:38
  • @Thilo A great idea. – Grim Feb 26 '19 at 08:40

1 Answers1

-4

Hm,

since lambdas can be executed in a different Thread. so, here variable car must be final.

Vikrant Kashyap
  • 6,398
  • 3
  • 32
  • 52
Grim
  • 1,938
  • 10
  • 56
  • 123
  • 1
    This does not provide an answer to the question. To critique or request clarification from an author, leave a comment below their post. - [From Review](/review/low-quality-posts/22310230) – barbsan Feb 26 '19 at 09:10
  • @barbsan The question is: `Why is it a bad idea?`. One side of the bad idea is that the variable car must be final. – Grim Feb 26 '19 at 09:14
  • 1
    If your lambda can be executed asynchronously then you have _far_ deeper problems than a `final`! Your data to log may change between when you call log and when the logging is done unless every structure you pass in a completely immutable. That doesn't even get into the visibility problems of passing things between threads that were not meant to be passed between threads. Finally, no pun intended, the requirement for variables used in a lambda to be effectively final has literally nothing to do with threads. Even if this comment could be classed as an answer, it is egregiously incorrect. – Boris the Spider Feb 27 '19 at 08:02
  • @BoristheSpider I would never ever log in a different thread! The idea is insane. – Grim Feb 27 '19 at 11:20
  • @BoristheSpider Any NPE stacktrace would be useless! – Grim Feb 27 '19 at 11:26