Skip to main content
Fixed option passing
Source Link
200_success
  • 145.7k
  • 22
  • 191
  • 481

For starters, indent your code consistently — the standard in Ruby is two spaces. That includes indenting the contents of your begin-rescue-end blocks.

Normally, I don't like to make such a huge fuss about indentation, but in this case I think it's highly important, because:

  1. Your program has a highly unusual outline (infinite loops and a function definition(!) inside an infinite loop)
  2. The stakes are high: if you misbehave, you could make a lot of people upset. Therefore, good software engineering practices should be used.

An outline like this would be more idiomatic for Ruby:

class AnswerBot
  ROOT = 'http://stackexchange.com'
  CHAT_ROOT = 'http://chat.stackexchange.com'

  def initialize(options)
    @agent = Mechanize.new
    @options = options
  end

  def login
    # Do stuff with @agent
    login_form = $agent.get('https://openid.stackexchange.com/account/login').forms.first
    login_form.email = @options[:email]
    # ...
    @fkey = @agent.get(@options[:chatroot]CHAT_ROOT + '/chats/join/favorite').forms.last.fkey
  end

  def fetch_answers
    # Make request to api.stackexchange.com
    # ...
    data['items'].each { |event| yield event }
    return (data['backoff'] || 0).to_i
  end

  def send_message(text, retries=5, backoff=40)
    # ...
  end
end

bot = AnswerBot.new(:access_token => ...,
                    :room_number = 12723,
                    :site => 'codereview',
                    :email => ...,
                    :password => ...)
loop {
  begin
    bot.login

    do
      backoff = bot.fetch_answers do |event|
        if ['answer_posted'].include?(event['event_type']) # <-- Is that right?
          bot.send_message(...)
        end
      end
    while sleep(40 + backoff)
  rescue => e
    puts "An error occurred."
    p e
  end
  puts "Bot restarted."
}

For starters, indent your code consistently — the standard in Ruby is two spaces. That includes indenting the contents of your begin-rescue-end blocks.

Normally, I don't like to make such a huge fuss about indentation, but in this case I think it's highly important, because:

  1. Your program has a highly unusual outline (infinite loops and a function definition(!) inside an infinite loop)
  2. The stakes are high: if you misbehave, you could make a lot of people upset. Therefore, good software engineering practices should be used.

An outline like this would be more idiomatic for Ruby:

class AnswerBot
  ROOT = 'http://stackexchange.com'
  CHAT_ROOT = 'http://chat.stackexchange.com'

  def initialize(options)
    @agent = Mechanize.new
    @options = options
  end

  def login
    # Do stuff with @agent
    login_form = $agent.get('https://openid.stackexchange.com/account/login').forms.first
    # ...
    @fkey = @agent.get(@options[:chatroot] + '/chats/join/favorite').forms.last.fkey
  end

  def fetch_answers
    # Make request to api.stackexchange.com
    # ...
    data['items'].each { |event| yield event }
    return (data['backoff'] || 0).to_i
  end

  def send_message(text, retries=5, backoff=40)
    # ...
  end
end

bot = AnswerBot.new(:access_token => ...,
                    :room_number = 12723,
                    :site => 'codereview',
                    :email => ...,
                    :password => ...)
loop {
  begin
    bot.login

    do
      backoff = bot.fetch_answers do |event|
        if ['answer_posted'].include?(event['event_type']) # <-- Is that right?
          bot.send_message(...)
        end
      end
    while sleep(40 + backoff)
  rescue => e
    puts "An error occurred."
    p e
  end
  puts "Bot restarted."
}

For starters, indent your code consistently — the standard in Ruby is two spaces. That includes indenting the contents of your begin-rescue-end blocks.

Normally, I don't like to make such a huge fuss about indentation, but in this case I think it's highly important, because:

  1. Your program has a highly unusual outline (infinite loops and a function definition(!) inside an infinite loop)
  2. The stakes are high: if you misbehave, you could make a lot of people upset. Therefore, good software engineering practices should be used.

An outline like this would be more idiomatic for Ruby:

class AnswerBot
  ROOT = 'http://stackexchange.com'
  CHAT_ROOT = 'http://chat.stackexchange.com'

  def initialize(options)
    @agent = Mechanize.new
    @options = options
  end

  def login
    # Do stuff with @agent
    login_form = $agent.get('https://openid.stackexchange.com/account/login').forms.first
    login_form.email = @options[:email]
    # ...
    @fkey = @agent.get(CHAT_ROOT + '/chats/join/favorite').forms.last.fkey
  end

  def fetch_answers
    # Make request to api.stackexchange.com
    # ...
    data['items'].each { |event| yield event }
    return (data['backoff'] || 0).to_i
  end

  def send_message(text, retries=5, backoff=40)
    # ...
  end
end

bot = AnswerBot.new(:access_token => ...,
                    :room_number = 12723,
                    :site => 'codereview',
                    :email => ...,
                    :password => ...)
loop {
  begin
    bot.login

    do
      backoff = bot.fetch_answers do |event|
        if ['answer_posted'].include?(event['event_type']) # <-- Is that right?
          bot.send_message(...)
        end
      end
    while sleep(40 + backoff)
  rescue => e
    puts "An error occurred."
    p e
  end
  puts "Bot restarted."
}
Source Link
200_success
  • 145.7k
  • 22
  • 191
  • 481

For starters, indent your code consistently — the standard in Ruby is two spaces. That includes indenting the contents of your begin-rescue-end blocks.

Normally, I don't like to make such a huge fuss about indentation, but in this case I think it's highly important, because:

  1. Your program has a highly unusual outline (infinite loops and a function definition(!) inside an infinite loop)
  2. The stakes are high: if you misbehave, you could make a lot of people upset. Therefore, good software engineering practices should be used.

An outline like this would be more idiomatic for Ruby:

class AnswerBot
  ROOT = 'http://stackexchange.com'
  CHAT_ROOT = 'http://chat.stackexchange.com'

  def initialize(options)
    @agent = Mechanize.new
    @options = options
  end

  def login
    # Do stuff with @agent
    login_form = $agent.get('https://openid.stackexchange.com/account/login').forms.first
    # ...
    @fkey = @agent.get(@options[:chatroot] + '/chats/join/favorite').forms.last.fkey
  end

  def fetch_answers
    # Make request to api.stackexchange.com
    # ...
    data['items'].each { |event| yield event }
    return (data['backoff'] || 0).to_i
  end

  def send_message(text, retries=5, backoff=40)
    # ...
  end
end

bot = AnswerBot.new(:access_token => ...,
                    :room_number = 12723,
                    :site => 'codereview',
                    :email => ...,
                    :password => ...)
loop {
  begin
    bot.login

    do
      backoff = bot.fetch_answers do |event|
        if ['answer_posted'].include?(event['event_type']) # <-- Is that right?
          bot.send_message(...)
        end
      end
    while sleep(40 + backoff)
  rescue => e
    puts "An error occurred."
    p e
  end
  puts "Bot restarted."
}