Using { } to segment large blocks of code to improve code-readability - Good practice?

Question

Hello,

I'm considering the option of using anonymous { } code blocks to logically distinguish "code blocks" inside the same method call, something that (theoretically) should improve readability of the code.

I'm wondering which of the following 2 code segments is better to your eyes?

Also, are the 2 code segments compile to the same bytecode?, In other words, can using { } hurt in any way the performance of the code?

Option 1: Code block without { } identation

 public static String serviceMatch(HttpServletRequest servletRequest, RequestTypeEnum requestTypeEnum, ...censorsed..., RequestStatistics requestStatistics) {
  Request request;

  // We get the parser that fits the ...censorsed..., effectively transforming the HTTPReqeuest to application local "Request*" object
  RequestParser parser = RequestParserFactory.getParser(...censorsed...);

  // Populate basic parameters, the "heavy" data will be lazy loaded
  request = parser.parse(servletRequest);

  // Instead of polluting the parsers let's put it here... (unless we identify meaningful justifications for the other alternative of changing RequestParser.parse() interface.
  request.requestType = requestTypeEnum;

  // Store the request statistics object on the request, so that we have access to it from all over the code
  request.requestStatistics = requestStatistics;



  // Update timestamp when request was parsed
  request.requestStatistics._1_end_parseRequest = System.currentTimeMillis();


  /*
   * ...censorsed...
   */
  MatchResult matchResult = Matcher.findMatch(...censorsed...);

  /*
   * ...censorsed...
   */
  String reply = ReplyFormatFactory.getFormatter(...censorsed...

  // Update timestamp when reply finished construction
  request.requestStatistics._6_end_formatReply = System.currentTimeMillis();

  return reply;
 }

Option 2: Code block with { } identation

 public static String serviceMatch(HttpServletRequest servletRequest, RequestTypeEnum requestTypeEnum, ...censorsed..., RequestStatistics requestStatistics) {
  Request request;

  /*
   * Request parsing block
   */
  {
   // We get the parser that fits the ...censorsed..., effectively transforming the HTTPReqeuest to application local "Request*" object
   RequestParser parser = RequestParserFactory.getParser(...censorsed...);

   // Populate basic parameters, the "heavy" data will be lazy loaded
   request = parser.parse(servletRequest);

   // Instead of polluting the parsers let's put it here... (unless we identify meaningful justifications for the other alternative of changing RequestParser.parse() interface.
   request.requestType = requestTypeEnum;

       // Store the request statistics object on the request, so that we have access to it from all over the code
   request.requestStatistics = requestStatistics;
  }



  // Update timestamp when request was parsed
  request.requestStatistics._1_end_parseRequest = System.currentTimeMillis();


  /*
   * ...censorsed...
   */
  MatchResult matchResult = Matcher.findMatch(...censorsed...);

  /*
   * ...censorsed...
   */
  String reply = ReplyFormatFactory.getFormatter(...censorsed...

  // Update timestamp when reply finished construction
  request.requestStatistics._6_end_formatReply = System.currentTimeMillis();

  return reply;
 }

Thanks for the review, Maxim.

Answer 1

If you're looking into adding extra { }'s within the same method just for the sake of readability, my advice would be to consider refactoring your method into several smaller methods.
These smaller methods have the advantage of being easier to understand by themselves, and being more reusable (if they are "loosely coupled"). See the single responsibility principle.

Answer 2

If you come to the state that it would be handy to put the brackets around some part of code (like in Option 2), you should move it to its own method. That's what improves readability.

By the way, I also think you don't really need to comment every single line of your code. For example the timestamp update is self-explanatory even without the comment.

Answer 3

I think this is a bit subjective, no right or wrong answer... my opinion is don't do it. Separate blocks of code with comment blocks that precede and explain why they are different, but don't use the braces. When I see braces, I immediately think there should be a leading if, while, or something... and not finding is is a little weird.

Answer 4

You should probably use separate methods instead. You can call the first block processRequest. Anyone who reads this code will be able to see which parameters are used, what data is returned, what it does (even without comments). Blocks don't provide such information.

Bytecode will likely be the same.

Answer 5

If you're developping in C# i would advise you to use #region ... #endregion instead for readability purpose.

Answer 6

I sometimes prefer to use the second option. That happens when extracting separate methods would lead to mess with multiple return parameters (that is, wrapping them in artificial object).

Answer 7

I don't generally add a brace-delimited block without some syntactic reason, but if a variable will only be needed within a limited scope, I'd rather created a nested scope than define the variable in the middle of a larger one (since in the latter case there's no clear indication when the variable goes out of 'useful' scope).

As for pulling out such a code block into another method, I think it's a good idea if the resulting method both (1) has a reasonable batch of parameters, and (2) can be given a name that describes its behavior as well as the actual code does. If using the method would require passing an excessive number of parameters, or if one would have to look at the code in the method to understand what its caller is doing, then I think it's better to use an anonymous scoping block.

Answer 8

Lighttpd has a comment blocks in configuration file, made in this style;

#{{{ module name
module.option = value;
module.option = value;
#}}} 

So you can just comment instead of {}'ing your code.

In Perl, anything inside { }, sub { } or eval { } will be evaluated; however, keeping a large amount of { } blocks inside some sub-routine is considered bad enough to push the code out in smaller parts;

$html .= eval { $val = &getNextPiece(); return $val; };

So the practice is known.

Answer 9

Braces are usually used to group statements for control structures and the like. I find them jarring when used for anything else.

If I have an overlong function that (for whatever reason) I don't want to split up, I break it apart into blocks with comments.

Answer 10

Braces { } have their purpose (even more in Java 7) and I think they are rarely used just for readability. Personally, if they are used like in Option 2 the first thing that comes to my mind is that, "Is this a static block?". Hence, I find Option 1 "more normal" and readable.

If you are really keen on sticking with one method and not refactoring this chuck of code as suggested by many here, then use comments as line separators instead. Something like:

    /* -------------------------------------------- */
    /* describe in detail here why you don't want to put this in another method */
    /* so other readers will know why! */

   // We get the parser that fits the ...censorsed..., effectively transforming the HTTPReqeuest to application local "Request*" object
   RequestParser parser = RequestParserFactory.getParser(...censorsed...);

   // Populate basic parameters, the "heavy" data will be lazy loaded
   request = parser.parse(servletRequest);

   // Instead of polluting the parsers let's put it here... (unless we identify meaningful justifications for the other alternative of changing RequestParser.parse() interface.
   request.requestType = requestTypeEnum;

       // Store the request statistics object on the request, so that we have access to it from all over the code
   request.requestStatistics = requestStatistics;
  }
   /* -------- END of confusing block ------------- */

IMHO, comments are probably the best in making codes readable.