views:

86

answers:

3

Hello, I have a text processing thing I'm doing in Ruby. Basically, I have to implement a simple state machine(with one character look-behind

My code at the moment looks like this:

text.each{ |c|
  ...
  ...
  ...
  ...
  if @state!=:some_state
    next
  end
  #processing stuff for if in :some_state mode
  ...
  ...
  ...
  ...
  ...
}

Is this proper? Or should it rather be implemented like

text.each{ |c|
  ...
  ...
  ...
  ...
  if @state==:some_state
    #processing stuff for if in :some_state mode
    ...
    ...
    ...
    ...
    ...
  end
}

Is there a right way or is it just preference? Which one blends more with "the ruby" way of doing things?

A: 

Do it the second way


Some schools of thought are opposed to things in various languages like next, retry, continue, and break, because they are just a little bit too much in the direction of the disrespected goto statement.

While I'm not opposed to their use, it's not a good idea to deliberately "spaghettify" the code when a structured downward-pointing construct will accomplish the same thing.

DigitalRoss
Wayne Conrad
+2  A: 

Totally agree with @DigitalRoss and I have seen people using next if there is complicated piece of code after some condition is being evaluated i.e.

 next if @state!=:some_state
 # some long complicated code

on the other hand if there is a simple operation that needs to be performed on the basis of some condition then I would prefer

 if @state == :some_state
   #call_a_method_to_do_something
 end

 OR

 call_a_method if @state == :some_state

Having said that, its bad practice to write long complicated code. If your code is clean and well designed then you would never have to use next within your code.

nas
Well it's a state machine with a lot of states, so I can only abstract so much away(and it is as DRY as I can make it), so I have no choice but to have over 50 lines of code *after* the `next` statement in question, so would that qualify as fair use?
Earlz
it's a judgement call - IMHO guard clauses are ok, but they belong at the beginning of a code block.
klochner
If you can get your code working easily with an `if` statement then still avoid `next`. However, if you have a 50 line of code within a single method then I think you should still give it a hard look as that code might be doing too many things. Try to abstract bits of it in separate methods. Methods or functions should be small, have same level of abstraction with single responsibility and either change the state of the object or return a value.
nas
I normally reserve my `next if predicate` use to the start of a block, just for clarity.
rampion
+1  A: 

I think that the example you gave doesn't really capture situation where doing next makes a difference. Consider the situation where you have multiple "next-points" in your code:

text.each do |c|
  next if @state == :state1
  ...
  next if @state == :state2
  ...
  next if @state == :state3
  ...
end

and compare it with if-variant:

text.each do |c|
  unless @state == :state1
    ...
    unless @state == :state2
      ...
      unless @state == :state3
        ...
      end
    end
  end
end

Although the first could be viewed as spaghetti-style by some purists, IMHO it is more readable than the latter.

Mladen Jablanović