From: masta Blasta <lists@...>
Date: 2012-06-28T05:19:16+09:00
Subject: Re: Is this good code

Marc Heiler wrote in post #1066407:
>> The conditional dependent 'end' is irritating me a lot. I don't mind
>> mid method break or return statements, but this just looks obnoxious.
>> I'm tempted to re-write the whole method to get rid of it.
>
> If it bothers your brain to look at it, definitely rewrite it so
> that your brain feels more comfortable.
>
> I myself use short conditionals usually only in block form.
>
> Example:
>
> def foo
>   invoke_this_method {
>     method_one
>     method_two
>   } unless @run_application
> end
>
> This is not that easy to read for others, and I don't do it often, but
> when I do it, I try to make it be the last part of the method.
>
> Having an:
>
> "end unless condition_is_true"
>
> in the middle of a method can be difficult to follow.
>
> I try to keep things as simple as possible usually though,
> so I rarely use a construct like this.


Digging through more code i found an even more convoluted example. Here
there is a double end conditional. One of them is for a block. How can
you conditionally end a block??

Sry for long line width


def rebuild_todo_items(todo_type, notify=false)
    #add case for when users are suspended to delete all their todo
items.
    case
    when todo_type == "pending term"
      skip_term_versions = Array.new
      self.todo_items.of_type("pending term").each do | tdi |
        if tdi.primary_object.state_owners.include?(self)
          skip_term_versions << tdi.primary_object
        else
          tdi.unassign_from_owner(self)
        end
      end
      self.term_versions_created.with_state("input_requested").each do |
tv |
        tv.build_todo_ownership(self) unless
skip_term_versions.include?(tv)
      end
      self.term_versions_created.with_state("rejected").each do | tv |
        tv.build_todo_ownership(self) unless
skip_term_versions.include?(tv)
      end
      self.functional_area_memberships.each do | fam |
        fa = fam.functional_area
        fa.terms.each do | t |
          if t.versions.latest.state == "pending" &&
!(skip_term_versions.include?(t.versions.latest))
            t.versions.latest.build_todo_ownership(self)
          end
        end if self.role_for(fa).include?("moderator") ||
self.role_for(fa).include?("manager") #-----------> GOTCHA
      end unless self.functional_area_memberships.empty? #----------> 
GOTCHA AGAIN?
    when todo_type == "report request"

                .......more code
    end
    self.save
  end

-- 
Posted via http://www.ruby-forum.com/.