Topic: Combining two similar helpers

Could anyone help me combine the following?

# TODO - merge with get_class
def status_pic(req)
  return image_tag("approved.png") if req.approved
  return image_tag("denied.png")   if req.denied
  return image_tag("question.png") if req.requested
  return ""
end

# TODO - merge with status_pic
def get_class(req)
  return "approved" if req.approved
  return "denied"   if req.denied
  return "question" if req.requested
  return ""
end

# What I'd like to have
# type = 1 for image type = 2 for string
def get_status(req, type = 1)
# ...
end

# Maybe something like this? ...
# Is DRY getting in the way of readability here?
def get_status(req, type = 1)
  [approved, denied, requested].each do |state|
    if req.state
      # If I did this I would have to change "question" to "requested"
      return image_tag("#{state}.png") if req.type == 1
      return state
    end
  end
end


TIA for any help

Re: Combining two similar helpers

I would make a method called status for the req that returns :approved, :denied, :requested.

def status_pic(req)
  image_tag("#{get_class(req)}.png")
end

def get_class(req)
  case req.status
  when :approved  then 'approved'
  when :denied    then 'denied'
  when :requested then 'question'
  end
end

def get_status(req, type = :image)
  if type == :image
    status_pic(req)
  else
    get_class(req)
  end
end


I'm not sure what do you want to accomplish exactly. If :requested doesn't need to be 'question' then you can remove the get_class method completely.

Re: Combining two similar helpers

Hmm sorry if this was unclear req is actually a model (Request) which among other things has three datetime attributes called approved, denied, and requested. Approved and denied are mutually exclusive. Requested can (but doesn't have to) overlap both approved and denied. This is why I check approved and denied first then requested. So I'm not sure about your code wouldn't work (there is no req.status). I think you may have picked that up from my pseudo-ruby where I called req.state but that was actually a block variable and I'm not even sure that would work.

Sorry for more poor explanation of the problem.

innu wrote:

I'm not sure what do you want to accomplish exactly.

Basically in one case I want to return a string (that I use for setting the class of a certain html element) and the other case I want to return an actual image. In both situations the logic is the same and I just thought that the code could be DRYer.

EDIT:
So you're code did give me this idea, but I'm not sure if it is any better because I have to call get_class twice. Let me know what you think. Is there a better way to write get_class?

def status_pic(req)
  return "" if get_class(req) == ""
  return image_tag("#{get_class(req)}.png")
end

def get_class(req)
  return "approved" if req.approved
  return "denied"   if req.denied
  return "requested" if req.requested
  return ""
end

Last edited by JackVandaL (2009-02-04 14:37:21)

Re: Combining two similar helpers

Hmm..
I think that I would make a method to the Request model:

def state
  return :approved  if self.approved
  return :denied    if self.denied
  return :requested if self.requested
end # these doesn't need to be symbols

It's now easier to check the request state somewhere else too.

So the status pic method can be just

def status_pic(req)
  if state = req.state
    image_tag("#{state}.png")
  else
    ''
  end
end

Last edited by innu (2009-02-06 14:45:54)

Re: Combining two similar helpers

Brilliant I'll try it out. Thanks innu.