is it good programming practice to put code statements in braces?

Question

I have never seen this being done anywhere in all the source code i've read in my life. If it is considered bad programming practice, there has to be a reason for it which i fail to understand. Also, I think it sometimes improves readability rather than worsening it. Here are a few places i've done it in my ruby code.

@pushButton.connect(SIGNAL :clicked) do (@select_file ||= Qt::FileDialog.new).show end

and

(tmr=Qt::Timer.new).connect SIGNAL :timeout do 
  @label.text = Qt::Application.translate("MainWindow", "The time right now is #{Time.now}", nil, Qt::Application::UnicodeUTF8)
end 
tmr.start(1000)

Answer 1

I think it's rather hard to read. I would prefer it slightly more verbose, but clearer:

tmr = Qt::Timer.new
tmr.connect SIGNAL :timeout do 
  @label.text = Qt::Application.translate("MainWindow", "The time right now is #{Time.now}", nil, Qt::Application::UnicodeUTF8)
end 
tmr.start(1000)

Answer 2

Code with too many logical statements crammed into 1 line is hard to read, because the reader essentially has to unwind it in their brain into a statement similar to troelskn's answer.

Other reasons it's less readabale:

• It makes the line length flow past the border of most text editor windows.
• It obscures individual steps you might need to see while debugging.

Answer 3

It's always a good idea to strive for simplicity wherever possible, and to that end it's best to state things in a straightforward manner. Declarations like that make it hard to determine where variables originate as they're embedded rather thoroughly inside the statement.

Declaring scoped variables within brackets is usually considered acceptable:

if (found = MyModel.find_by_pigeon_id(params[:pigeon_id]))
  # Variable 'found' used only within this block
end

# Ruby variables will persist here, but in many languages they are out of scope

A more verbose version actually has implications:

found = MyModel.find_by_pigeon_id(params[:pigeon_id])
if (found)
  # Variable 'found' can be used here
end

# Implies 'found' may be used here for whatever reason

It's always nice to be able to scan up through the program and see, quite clearly, all the variables as they're declared. Hiding things serves no purpose other than to frustrate people.

Ruby is a lot more relaxed than many other languages in terms of how much you can get away with. Some languages will punish you severely for complicating things because a tiny mistake in declaration or casting can have enormous ramifications. That doesn't mean you should take full advantage of that at every opportunity.

Here's how I'd advocate implementing your first example:

# Ensure that @select_file is defined
@select_file ||= Qt::FileDialog.new

@pushButton.connect(SIGNAL(:clicked)) do
  # Block is split out into multiple lines for clarity
  @select_file.show
end

The second:

# Simple declaration, variable name inherited from class name, not truncated
timer = Qt::Timer.new

timer.connect(SIGNAL(:timeout)) do 
  # Long parameter list is broken out into separate lines to make it clear
  # what the ordering is. Useful for identifying accidentally missing parameters.
  @label.text = Qt::Application.translate(
    "MainWindow",
    "The time right now is #{Time.now}",
    nil,
    Qt::Application::UnicodeUTF8
  )
end

timer.start(1000)

I've found that the most complicated programs often look the simplest, as they're written by people with lots of experience who know how to express things in a straightforward manner.

Interestingly enough, some of the simplest programs are often the most complicated as they're written by novices who are either grandstanding and showing off or are digging themselves into a deep ditch and keep throwing code at the problem in the hopes of fixing it.

Answer 4

I think you mean parens?

Brace: {

Paren: (

Your first example becomes slightly more readable if you use braces instead of do/end---conventionally you use braces if a block is on a single line, with do/end for multi-line blocks.

@pushButton.connect(SIGNAL :clicked) { (@select_file ||= Qt::FileDialog.new).show }

The second example I've seen a lot of in java/c, but I've actually seen it done more functionally in ruby:

Qt::Timer.new.connect SIGNAL :timeout do 
  @label.text = Qt::Application.translate("MainWindow", "The time right now is #{Time.now}", nil, Qt::Application::UnicodeUTF8)
end.start(1000)

But you then don't have a reference to the object after starting it.