All the Perl that's Practical to Extract and Report

Lies, damn lies and end-of-block comments

schwern on 2008-09-26T20:59:57

Ever since the small audience revolt at YAPC::NA over my assertion during the "Skimmable Code" talk that end-of-block comments should be removed [1], I have taken great glee in finding lying end-of-block comments and expelling them. The latest is in reentr.h in the Perl source.

Here's how it is written now:

#ifdef HAS_LOCALTIME_R
#  if defined(PERL_REENTR_API) && (PERL_REENTR_API+0 == 1)
#   undef localtime
#   if !defined(localtime) && LOCALTIME_R_PROTO == REENTRANT_PROTO_S_TS
#       define localtime(a) (L_R_TZSET localtime_r(a, &PL_reentrant_buffer->_localtime_struct) ? &PL_reentrant_buffer->_localtime_struct : 0)
#  endif /* if defined(PERL_REENTR_API) && (PERL_REENTR_API+0 == 1) */
#   if !defined(localtime) && LOCALTIME_R_PROTO == REENTRANT_PROTO_I_TS
#       define localtime(a) (L_R_TZSET localtime_r(a, &PL_reentrant_buffer->_localtime_struct) == 0 ? &PL_reentrant_buffer->_localtime_struct : 0)
#  endif /* if defined(PERL_REENTR_API) && (PERL_REENTR_API+0 == 1) */
#   endif /* HAS_LOCALTIME */
#endif /* HAS_LOCALTIME_R */


Over and over and over again. How many lying end-of-block comments can you count? [2]

#ifdef HAS_LOCALTIME_R
#  if defined(PERL_REENTR_API) && (PERL_REENTR_API+0 == 1)
#   undef localtime
#   if !defined(localtime) && LOCALTIME_R_PROTO == REENTRANT_PROTO_S_TS
#       define localtime(a) (L_R_TZSET localtime_r(a, &PL_reentrant_buffer->_localtime_struct) ? &PL_reentrant_buffer->_localtime_struct : 0)
#   endif
#   if !defined(localtime) && LOCALTIME_R_PROTO == REENTRANT_PROTO_I_TS
#       define localtime(a) (L_R_TZSET localtime_r(a, &PL_reentrant_buffer->_localtime_struct) == 0 ? &PL_reentrant_buffer->_localtime_struct : 0)
#   endif
#  endif
#endif /* HAS_LOCALTIME_R */


By removing all that redundant code now you can see the block structure. Throw in some vertical whitespace, normalize the indentation, and...

#ifdef HAS_LOCALTIME_R
#  if defined(PERL_REENTR_API) && (PERL_REENTR_API+0 == 1)
#    undef localtime
#
#    if !defined(localtime) && LOCALTIME_R_PROTO == REENTRANT_PROTO_S_TS
#      define localtime(a) (L_R_TZSET localtime_r(a, &PL_reentrant_buffer->_localtime_struct) ? &PL_reentrant_buffer->_localtime_struct : 0)
#    endif
#
#    if !defined(localtime) && LOCALTIME_R_PROTO == REENTRANT_PROTO_I_TS
#      define localtime(a) (L_R_TZSET localtime_r(a, &PL_reentrant_buffer->_localtime_struct) == 0 ? &PL_reentrant_buffer->_localtime_struct : 0)
#    endif
#
#  endif
#endif /* HAS_LOCALTIME_R */


I can almost read that! Break up those overly long lines and...

#ifdef HAS_LOCALTIME_R
#  if defined(PERL_REENTR_API) && (PERL_REENTR_API+0 == 1)
#    undef localtime
#
#    if !defined(localtime) && LOCALTIME_R_PROTO == REENTRANT_PROTO_S_TS
#      define localtime(a)      (                                               \
         L_R_TZSET localtime_r(a, &PL_reentrant_buffer->_localtime_struct)      \
           ? &PL_reentrant_buffer->_localtime_struct : 0                        \
       )
#    endif
#
#    if !defined(localtime) && LOCALTIME_R_PROTO == REENTRANT_PROTO_I_TS
#      define localtime(a)      (                                               \
         L_R_TZSET localtime_r(a, &PL_reentrant_buffer->_localtime_struct) == 0 \
           ? &PL_reentrant_buffer->_localtime_struct : 0                        \
       )
#    endif
#
#  endif
#endif /* HAS_LOCALTIME_R */


...it makes sense, as much as C defines ever do.

[1] They are red flag for overly-complex code, a maintenance hassle, obscuring the very block structure it's supposed to be commenting and should be removed.

[2] 3 out of 4.

We have the same problem in our codebase

Alias on 2008-09-27T06:47:45

Someone at some point in history decided PerlTidy'ing the entire 250,000 line codebase would be a good idea, and that automatically appending end-of-block comments would probably be a good idea too.

I get it would be trivial to write a PPI search/replace for it... hrm...

Re:We have the same problem in our codebase

schwern on 2008-09-27T19:27:18

What perltidy giveth, perltidy can taketh away. Use the -dcsc option to delete "closing side comments".

At least if perltidy is writing your end-of-scope comments for you they'll stay up to date.

Evolution of structural competence

brian_d_foy on 2008-09-27T21:28:20

I like to say this progression of coding style is going from forming letters, to writing words, to composing sentences, comprising sentences in paragraphs, and so on.

Most people struggle to compose sentences, so paragraphs are right out.

And, blank lines might have been a hassle back in the day when you were limited by the hardware to 24 rows, but how anyone still knows about that today is baffling. Throw some whitespace in your code and then see how much easier it is to read. (or take out the fomatting whitespace in your favorite novel and see how hard it is to read).