Should a "static final Logger" be declared in UPPER-CASE?

Question

In Java, static final variables are constants and the convention is that they should be in upper-case. However, I have seen that most people declare loggers in lower-case which comes up as a violation in PMD.

e.g:

private static final Logger logger = Logger.getLogger(MyClass.class);

Just search googleor SO for "static final logger" and you will see this for yourself.

Should we be using LOGGER instead?

Solution 1

The logger reference is not a constant, but a final reference, and should NOT be in uppercase. A constant VALUE should be in uppercase.

private static final Logger logger = Logger.getLogger(MyClass.class);

private static final double MY_CONSTANT = 0.0;

Solution 2

To add more value to crunchdog's answer, The Java Coding Style Guide states this in paragraph 3.3 Field Naming

Names of fields being used as constants should be all upper-case, with underscores separating words. The following are considered to be constants:

1. All static final primitive types (Remember that all interface fields are inherently static final).
2. All static final object reference types that are never followed by "." (dot).
3. All static final arrays that are never followed by "[" (opening square bracket).

Examples:

MIN_VALUE, MAX_BUFFER_SIZE, OPTIONS_FILE_NAME

Following this convention, logger is a static final object reference as stated in point 2, but because it is followed by "." everytime you use it, it can not be considered as a constant and thus should be lower case.

Solution 3

From effective java, 2nd ed.,

The sole exception to the previous rule concerns “constant fields,” whose names should consist of one or more uppercase words separated by the underscore character, for example, VALUES or NEGATIVE_INFINITY. A constant field is a static final field whose value is immutable. If a static final field has a primitive type or an immutable reference type (Item 15), then it is a constant field. For example, enum constants are constant fields. If a static final field has a mutable reference type, it can still be a constant field if the referenced object is immutable.

In summary, constant == static final, plus if it's a reference (vs. a simple type), immutability.

Looking at the slf4j logger, http://www.slf4j.org/api/org/slf4j/Logger.html

It is immutable. On the other hand, the JUL logger is mutable. The log4j logger is also mutable. So to be correct, if you are using log4j or JUL, it should be "logger", and if you are using slf4j, it should be LOGGER.

Note that the slf4j javadocs page linked above has an example where they use "logger", not "LOGGER".

These are of course only conventions and not rules. If you happen to be using slf4j and you want to use "logger" because you are used to that from other frameworks, or if it is easier to type, or for readability, go ahead.

Solution 4

I like Google's take on it (Google Java Style)

Every constant is a static final field, but not all static final fields are constants. Before choosing constant case, consider whether the field really feels like a constant. For example, if any of that instance's observable state can change, it is almost certainly not a constant. Merely intending to never mutate the object is generally not enough.

Examples:

// Constants
static final int NUMBER = 5;
static final ImmutableList<String> NAMES = ImmutableList.of("Ed", "Ann");
static final Joiner COMMA_JOINER = Joiner.on(',');  // because Joiner is immutable
static final SomeMutableType[] EMPTY_ARRAY = {};
enum SomeEnum { ENUM_CONSTANT }

// Not constants
static String nonFinal = "non-final";
final String nonStatic = "non-static";
static final Set<String> mutableCollection = new HashSet<String>();
static final ImmutableSet<SomeMutableType> mutableElements = ImmutableSet.of(mutable);
static final Logger logger = Logger.getLogger(MyClass.getName());
static final String[] nonEmptyArray = {"these", "can", "change"};

Solution 5

If you are using an automated tool to check your coding standards and it violates said standards then it or the standards should be fixed. If you're using an external standard, fix the code.

The convention in Sun Java is uppercase for public static constants. Obviously a logger is not constant, but represents a mutable thing ( otherwise there would be no point calling methods on it in the hope that something will happen ); there's no specific standard for non-constant final fields.

View more solutions

Comments

[Daniel Hári]

PMD or Checkstyle are pre-mature naive attempts to increase readability but they cause more harm than benefit. A most readable style can change case by case based on the context. See Guava, or the JDK src, those does not follow any strict style template, but made by professionals it's unquestionable. example: DelegatedExecutorService @ docjar.com/html/api/java/util/concurrent/Executors.java.html

[Kenston Choi]

Sonar Rules (rules.sonarsource.com/java/tag/convention/RSPEC-1312) also has it as private static final Logger LOGGER = LoggerFactory.getLogger(Foo.class);

[CppDude]

Why are you saying the logger is not constant? It seems constant indeed. The logging is produces is a side-effect of calling its methods, but don't change its observable state. Did I miss something?

[user9562401]

Catching Throwable is bad practice, unless you log and rethrow it. Remember Errors: OutOfMemeoryError, etc. Event Exception is not so safe to be catched and handled by yourself in multi-thread applications.

[Brett]

Check the API. It does have an add/get pair of methods. But you reasoning is flawed anyway. Logging is observable (otherwise, what's the point).

[dogbane]

Eclipse syntax is: Logger.getLogger(${enclosing_type}.class);

[Troj]

The logger is not a constant as it is a reference to an object. Constants are values that can't be changed. The object reference is final (so the reference to it can't be changed, e.g. swapped with something else or set to null) but the object itself can.

[IAdapter]

Or dont use PMD, they are always wrong and your code is perfect

[Jeffrey Blattman]

you are confusing the terms constant and final. in java, final != non-mutable, it means that the object referenced by the field declared final won't change.

[Pete Kirkham]

@farble1670 I am well aware of the difference. The standard naming (up to 5 at least) was that constants - values capable of being inlined by the compiler - were uppercase. A final reference to a mutable object is not a constant, therefore having it in uppercase to warn you to recompile dependent classes if you change it is not appropriate.

[robert]

Based on this reasoning then checkstyle's simplistic definition is inappropriate right?

[robert]

Best definition I've seen for this yet. Linked doc seems to have moved here's the update cs.bilgi.edu.tr/pages/standards_project/…

[Jeffrey Blattman]

i don't know check style's rules. if it's simply insisting that any static final should be upper-case, then yes, that's wrong.

[Jeffrey Blattman]

static final references are constants if they are immutable. by this logic, you would never have constant strings because any static final string is a reference.

[Jeffrey Blattman]

"Obviously a logger is not constant, but represents a mutable thing ( otherwise there would be no point calling methods on it in the hope that something will happen)"- not true. the logger could be immutable and calling some "log" method would not necessarily change the state of the object.

[Pete Kirkham]

@JeffreyBlattman if it was never the case that the logger represented a mutable thing, you wouldn't bother with having a logger at all - all the calls could be removed from the system without any change in observable behaviour. An object representing a mutable thing is not necessarily itself mutable - a logger might represent logging to the console, which changes the text on the screen, but can be implemented with an immutable object which calls sop. You seem to be arguing that because a final reference might sometimes be to an immutable object, it should be in upper case - is that so?

[Jeffrey Blattman]

@PeteKirkham yes, that's what i'm saying, and it's what Effective Java says as well. but hey, that josh bloch guy probably doesn't know what he's talking about right? in java, immutability means that state of the object is unchanging after construction. e.g., String is immutable, but you can still call methods on it that return new String objects. the point is that none of the methods modify the internal state of the object. please don't try to invent some other definition of immutability, it's confusing.

[Pete Kirkham]

@JeffreyBlattman I do not agree that all final references should be upper case, but you are free to adopt whatever coding standards you like. I am sorry that you find the difference between 'mutable object' and 'object which represents a mutable thing' confusing; one example may be your back account number, which itself does not change, but is used to access a variable balance. Look up for the difference between signifier and significand for more details, or an introduction to Leibnitz's monads for how an immutable thing can represent mutability.

[Aleksander Adamowski]

But java.lang.String is immutable and a special kind of class anyway (see String.intern(), documentation about the Sring pool etc.)

[Jeffrey Blattman]

immutable means the state of the object cannot change after construction. see my post below. loggers are not necessarily mutable.

[dogbane]

I don't get point 2. What is an example of an object type that is never followed by a dot. All object types inherit from Object and you can call a method such as .equals on them.

[cbliard]

You are right. And when looking at some Java constants like Boolean.TRUE, Boolean.FALSE, TimeUnit.MINUTES, String.CASE_INSENSITIVE_ORDER or Collections.EMPTY_LIST, they may be followed by . as well.

[Roman Ivanov]

Does anybody have "Java Coding Style Guide" stored locally? Looks like it was removed from internet resources. Please share document,as we try add support of it in Checkstyle github.com/checkstyle/checkstyle/issues/23.

[Roman Ivanov]

if somebody still care about this problem, please share ideas at github.com/checkstyle/checkstyle/issues/23, to distinguish where demand upper case and where not.

[cbliard]

@RomanIvanov I found it again here: scribd.com/doc/15884743/Java-Coding-Style-by-Achut-Reddy written by Achut Reddy, last update May 30, 2000

[Xairoo]

I believe the purpose of 2 is to designate that only classes that are meant to be compared to are considered constants. The class is not meant to be 'used'. I know always cringe when I see SOME_CLASS.doStuff(). It's just fugly coding. The only problem with this is in the common case of a constant object (String being the common example) which is only meant for comparison, but to avoid null checks, yoda style coding is used and thus equals() is called on the constant. I think I would make this the one caveat to 2.

[Dan]

@JeffreyBlattman Does immutable really mean the internal state of the object cannot change or that the user may not alter the state? Those are two very distinct notions.

[Jeffrey Blattman]

@Jeach i don't think immutability is concerned with how the state changes, only that it does. moreover, what's a user? the external user running the program? would you make a distinction between state being modified by a user pressing a button, and it being modified by a timer firing at some random interval? (i don't think so).

[keiki]

If you always need to exclude a check every time, then the check doesn't make sense.

[Craig van Wilson]

Couldn't agree more - however... its useful to know the exclusion comment

[javaPlease42]

@RomanIvanov The CheckStyle check on this drives me nuts. Yes I will do that. See this post

[Roman Ivanov]

@javaPlease42 , yes you are right , Checkstyle need to handle that please open issue on Checkstyle's issue tracker, please reference a text from answer below to prove that Constant definition is not that simple.

[Chetan Narsude]

We had similar debate at work. This thread helped to put some clarity on the issue but I also used the following example to make a case for an exception to the people who were proponent of upper case. Does anyone know why java has serialVersionUID as opposed to SERIAL_VERSION_UID? private static final long serialVersionUID = 201208271956L;

[ayahuasca]

I think the first sentence sums this up succintly: "Every constant is a static final field, but not all static final fields are constants." It's easy to use mechanical thinking and just have every static final field in uppercase (and I have been doing this until now) but this is to miss the subtlety of the language.

[Mark]

While this answer appears to be getting at static final immutable references, it totally misses the mark (esp. with #2). As mentioned in the comments above, ANY non-primitive value can be followed by a dot.

[Costi Ciudatu]

How exactly is the Logger interface immutable? Only a final class (like String or Integer) can guarantee immutability. Even if you can't find any mutable implementation of the SLF4J Logger, no one can stop you from writing one yourself.

[Jeffrey Blattman]

Because the methods in the interface do not allow mutation inherently. You are right though you could implement the interface to have mutatable side effects.

[Qix - MONICA WAS MISTREATED]

That's not the point. The point is that you shouldn't be doing anything to the stored object reference. That means using a Logger object (e.g. log.info(...)) is a violation of rule 2 since log is followed by a dot within that invocation. Since you can't mark methods as const like you can in C++, it's assumed all methods mutate the object and thus are not constants.

[Marvin]

Simple, objective and makes sense most of the time. This may not be a perfect convention but imo it's the best solution!

[Daniel Hári]

Check style rules are NOT MATURE enough to implicate readability. Readability can't achieved by templating a style, readabability can differs case by case based on the context. See the JDK code, it does not follow any style template, and made by professionals, that shows something.

[Daniel Hári]

If the "strict conventions" of CheckStyle or PMD helps, then why Guava, and JDK sources does not have ANY applied common style? For example their source have plenty of full inlined blocks where needed. Readability is context dependent, so using strict styling conventions for everything destroys context based decisions, so decreases readability.

[Jeffrey Blattman]

@DanielHári um, what? Are you really arguing for no style and just letting each dev do what they feel looks best? The JDK absolutely does have a style. Yes, the people that wrote it are not perfect and there are some places where they break their own rules.

[Jeffrey Blattman]

According to that quote, it boils down to if the field "really feels" like a constant. We're engineers, not psychiatrists.

[Daniel Hári]

Yes, my point is to less stupid automated rule and more subjective style that can't be automated but can be more appropriate. I hate rules.

[Jeffrey Blattman]

"Consider...if it really feels like a constant". Someone's feelings really should not enter into the field of engineering.

[Jeffrey Blattman]

@CostiCiudatu's comment about final classes is a good one. If I could guess, I assume the author or Effective Java would say that while you could be passed an interface for which you don't know the immutability, most often you are constructing the static final object yourself. You know what concrete instance it is, and you know if it's immutable. If you don't know, you should because using it properly demands that. Think about how your use of String would change if you didn't know its mutability.

[Kenston Choi]

Then in Guava's code they have it as private static final Logger logger = Logger.getLogger(Finalizer.class.getName());