1

The task is to check if a contact page exists and navigate to it. For the websites not in english, the method looks for an english page and then restarts to check for a contact page.

My conditional works fine, but I figured there must be a better way to do this:

  # First, I set the @url variable during Booleans.   
  # Checks are either to see if a link exists or if a page exists,
  # (aka no 404 error).
  #
  # Here are two examples:

  # Boolean, returns true if contact link is present.
  def contact_link?
    @url = link_with_href('contact')

    !@url.nil?
  end

  # True if contact page '../contact' does NOT get a 404 error.
  def contact_page?
    @url = page.uri.merge('../contact').to_s
    begin
      true if Mechanize.new.get(@url)
    rescue Mechanize::ResponseCodeError
      false
    end
  end

  # #
  # Now go to the correct page, based off of checks.
  #
  def go_to_contact_page
    1.times do
      case # No redo necessary.
      when contact_link? # True if hyperlink exists
        get(@url)
      when contact_page? # False if 404 error
        get(@url)
      else # Redo is now necessary.
        if english_link? # True if hyperlink exists
          get(@url)
          redo
        elsif en_page? # False if 404 error
          get(@url)
          redo
        elsif english_page? # False if 404 error
          redo
        end
      end
    end
  end

There are a couple things to draw your attention to:

  1. Is 1.times do the best way to do a single redo? Would begin be better?

  2. Understanding that I set the @url variable in each of these checks, there seems to be redundancy in get(@url) in the conditional branch. Is there a more succinct way?

  3. I am writing redo three times which also seems redundant. Is there a way to call it once and still set the @url variable?

Thanks for the help!

pguardiario
  • 53,827
  • 19
  • 119
  • 159
binarymason
  • 1,351
  • 1
  • 14
  • 31
  • Not sure I fully understand the question, but you can combine cases in a `case` statement with a comma (e.g. `when contact_link?, contact_page?`). You also don't need the `else` before the `if english_link?` block. That can be factored into your case statment – Josh Bodah Oct 09 '15 at 11:35

1 Answers1

1

Something like this is more readable and dry

def english_contact_page
  ..
rescue
  nil
end

def contact_page
  ..
rescue
  nil
end

def get_page
  @url = link_with_href('contact')
  return nil if @url.nil?
  contact_page || english_contact_page  # left side is evaluated first
rescue
  nil
end
peter
  • 41,770
  • 5
  • 64
  • 108