checkstyle rules: naming for ThreadLocal variables

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

checkstyle rules: naming for ThreadLocal variables

Simon Kitching
Hi All,

I have a fairly trivial question for you all.

The normal checkstyle rule for a static final variable requires the name
to be ALL_UPPERCASE_WITH_UNDERSCORES. There are two problem cases
though:
(1) private static final Log log = LogFactory.getLog(...);
(2) private static final ThreadLocal someVar = new ThreadLocal();

It's easy enough to tweak the rules to specifically allow "log" as a
special case constant name. In fact, already done.

Handling (2) is trickier. Note that it's just one case of a more generic
problem: constant references to mutable objects; however we just don't
do that for anything other than ThreadLocals. The options I can think of
are:

(a) Disable all checkstyle name checks for constant variables

(b) Require threadlocals to be ALL_CAPS. This does mean that code like
this then exists:
  private static final ThreadLocal HTTP_SERVLET_REQUEST
    = new ThreadLocal();

  ...

  if (someCondition)
  {
    HTTP_SERVLET_REQUEST.set(value);
  }

This looks a bit odd to me. I suppose it is a constant "pointer" to the
variable, but calling SET on an ALL_CAPS var still feels odd. Maybe
using a convention like
  HTTP_SERVLET_REQUEST_MUTABLE.set(...)
makes it a *little* less painful to look at, but that's pretty verbose.

(c) use a naming convention like requiring the name to end in
ThreadLocal but otherwise allow camelCase:

  private static final ThreadLocal httpServletRequestThreadLocal
     = new ThreadLocal();

  ....
  httpServletRequestThreadLocal.set(value);

Possibly the special suffix could be "Mutable" rather than
"ThreadLocal". More generic, but I don't know if we need that extra
flexibility..

(d) omit the final for threadlocal declarations. This does mean that
code can theoretically change which var is used, screwing things up. It
would be a pretty unlikely bug. But this is opening a potential hole
just in order to keep checkstyle happy.

Any other ideas?

My preferences in order would be (c), (b), (a).


Regards,
Simon

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: checkstyle rules: naming for ThreadLocal variables

Scott O'Bryan
I don't know why b is an issue.  The Caps- case notation I've always  
taken to be private statics rather then non-immutible constants.  I  
know in Trinidad, we use "LOG" for logs and I suspect the same for  
thread locals.

On Jul 4, 2008, at 2:04 PM, simon <[hidden email]> wrote:

> Hi All,
>
> I have a fairly trivial question for you all.
>
> The normal checkstyle rule for a static final variable requires the  
> name
> to be ALL_UPPERCASE_WITH_UNDERSCORES. There are two problem cases
> though:
> (1) private static final Log log = LogFactory.getLog(...);
> (2) private static final ThreadLocal someVar = new ThreadLocal();
>
> It's easy enough to tweak the rules to specifically allow "log" as a
> special case constant name. In fact, already done.
>
> Handling (2) is trickier. Note that it's just one case of a more  
> generic
> problem: constant references to mutable objects; however we just don't
> do that for anything other than ThreadLocals. The options I can  
> think of
> are:
>
> (a) Disable all checkstyle name checks for constant variables
>
> (b) Require threadlocals to be ALL_CAPS. This does mean that code like
> this then exists:
>  private static final ThreadLocal HTTP_SERVLET_REQUEST
>    = new ThreadLocal();
>
>  ...
>
>  if (someCondition)
>  {
>    HTTP_SERVLET_REQUEST.set(value);
>  }
>
> This looks a bit odd to me. I suppose it is a constant "pointer" to  
> the
> variable, but calling SET on an ALL_CAPS var still feels odd. Maybe
> using a convention like
>  HTTP_SERVLET_REQUEST_MUTABLE.set(...)
> makes it a *little* less painful to look at, but that's pretty  
> verbose.
>
> (c) use a naming convention like requiring the name to end in
> ThreadLocal but otherwise allow camelCase:
>
>  private static final ThreadLocal httpServletRequestThreadLocal
>     = new ThreadLocal();
>
>  ....
>  httpServletRequestThreadLocal.set(value);
>
> Possibly the special suffix could be "Mutable" rather than
> "ThreadLocal". More generic, but I don't know if we need that extra
> flexibility..
>
> (d) omit the final for threadlocal declarations. This does mean that
> code can theoretically change which var is used, screwing things up.  
> It
> would be a pretty unlikely bug. But this is opening a potential hole
> just in order to keep checkstyle happy.
>
> Any other ideas?
>
> My preferences in order would be (c), (b), (a).
>
>
> Regards,
> Simon
>
Loading...