drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel Barclay <dbarc...@maprtech.com>
Subject Re: dropping qualified names in logger declarations
Date Mon, 27 Jul 2015 21:52:35 GMT
 >  I can't seem to find any guidance for this particular issue.

Yeah; I wouldn't expect anything specific to our peculiar pattern,
other than a general endorsement of usually using imports and usually
not using fully-qualified names.

Is there any reason not to go with imports and non-qualified names
(with the parts I mentioned below to retain the easy commenting/
uncommenting to avoid unused imports and loggers)?

If there's concern about having loggers in both styles for a long
interim period:  I think I could convert the declarations pretty
rapidly (using some Emacs regular-expression bulk replacement), as
long as I can get someone to approve and merge in the changes as
quickly as we want the inconsistency to go away.


Chris Westin wrote:
> I tried to follow up with Hakim's suggestion of consulting the checkstyle
> rules I expect to use (I've suggested before that we should start with
> Google's rules as a basis, and make a few tweaks), but unfortunately, on
> that day last week, SourceForge was down (that's where the rules are
> hosted). It's finally back, so here they are
> http://checkstyle.sourceforge.net/google_style.html . I can't seem to find
> any guidance for this particular issue.
> On Wed, Jul 22, 2015 at 10:51 AM, Daniel Barclay <dbarclay@maprtech.com>
> wrote:
>> Chris Westin wrote:
>>> For the special case of the logger, I kind of like it this way, because I
>>> can turn it off just by commenting out a single line (to get rid of
>>> unreferenced variable warnings),or add it by pasting in or uncommenting a
>>> single line. In either case I don't have to worry about removing or adding
>>> the import line separately, which can be quite far away if there are a lot
>>> of imports.
>> Why not use the modern Java feature intended for cases like this:  have
>> a @SuppressWarnings("unused") annotation on the logger member
>> declaration if the declaration has been added but the member isn't used
>> yet?
>> Then:
>> - We can still avoid unused-variable warnings for logger members that
>>    have already been declared before there are any uses.
>> - We no longer have to move up top to adjust an already-existing logger
>>    declaration when adding a logger use down in the code.
>>    (Yes, we should remove (or comment out) the annotation if the change
>>    isn't temporary, but we don't have to do that immediately just to
>>    continue compiling.)
>> - We can now use code completion when adding the first logger call to a
>>    previously unused logger, since the declaration is real (not just a
>>    comment).
>> - We can still comment out/uncomment only a single line (the annotation)
>>    to switch between the no-logger-uses and some-logger-use cases.  (That
>>    is, if you don't want to have to re-add the suppression annotation if
>>    the last use of a logger is removed later, you can comment out, rather
>>    than delete, the annotation when the first logger use it added.
>> - We no longer have to either go adjust imports or use qualified names
>>    to avoid having to adjust imports.
>> - We stop having unnecessarily qualified names in the code.
>> - and, finally ...:
>> - We stop having those names' extra visual clutter and length, which
>>    makes it harder to notice when the class literal ends up wrong.
>>    (Note the mention of "pasting" above.)
>> Daniel
>>> On Tue, Jul 21, 2015 at 6:12 PM, Daniel Barclay <dbarclay@maprtech.com>
>>> wrote:
>>>   For logger member declarations, can we drop the current pattern of using
>>>> qualified names (like this:
>>>>     private static final org.slf4j.Logger logger =
>>>> org.slf4j.LoggerFactory.getLogger(StoragePluginRegistry.class);
>>>> ) and allow using imports and non-qualified names (as we do for almost
>>>> everything else)?
>>>> Using qualified names adds a lot of visual noise, and pushes the class
>>>> literal farther to the right, making it easier to fail to notice that
>>>> it doesn't match the containing class.
>>>> Thanks,
>>>> Daniel
>>>> --
>>>> Daniel Barclay
>>>> MapR Technologies
>> --
>> Daniel Barclay
>> MapR Technologies

Daniel Barclay
MapR Technologies

View raw message