# Chat bot for posting recent answers

I am the half-robot side of syb0rg that will be posting the recent answers of Code Review to the CR Answers chatroom. Here is the list of review suggestions I would like, in order of preference:

1. Efficiency (with API requests, speed of login and posting answers, etc.)
2. Security issues
3. Best practices

For feature requests regarding the chat bot, please see this meta post.

Any and all reviews are acceptable however. Don't be too harsh please, this is one of my first times using Ruby.

ACCESS_TOKEN = '<insert key>'
# https://stackexchange.com/oauth/dialog?client_id=2666&redirect_uri=http://keyboardfire.com/chatdump.html&scope=no_expiry
$root = 'http://stackexchange.com'$chatroot = 'http://chat.stackexchange.com'
$room_number = 12723 site = 'codereview' email = '<insert email>' password = '<insert password>' require 'rubygems' require 'mechanize' require 'json' require 'net/http' loop { begin$agent = Mechanize.new
$agent.agent.http.verify_mode = OpenSSL::SSL::VERIFY_NONE login_form =$agent.get('https://openid.stackexchange.com/account/login').forms.first
$agent.submit login_form, login_form.buttons.first puts 'logged in with SE openid' meta_login_form =$agent.get($root + '/users/login').forms.last meta_login_form.openid_identifier = 'https://openid.stackexchange.com/'$agent.submit meta_login_form, meta_login_form.buttons.last
puts 'logged in to root'

chat_login_form = $agent.get('http://stackexchange.com/users/chat-login').forms.last$agent.submit chat_login_form, chat_login_form.buttons.last
puts 'logged in to chat'

$fkey =$agent.get($chatroot + '/chats/join/favorite').forms.last.fkey puts 'found fkey' def send_message text loop { begin resp =$agent.post("#{$chatroot}/chats/#{$room_number}/messages/new", [['text', text], ['fkey', $fkey]]).body success = JSON.parse(resp)['id'] != nil return if success rescue Mechanize::ResponseCodeError => e puts "Error: #{e.inspect}" end puts 'sleeping' sleep 3 } end puts$ERR ? "An unknown error occurred. Bot restarted." : "Bot initialized."

last_date = 0
loop
{
uri = URI.parse "https://api.stackexchange.com/2.2/events?pagesize=100&since=#{last_date}&site=#{site}&filter=!9WgJfejF6&key=thqRkHjZhayoReI9ARAODA((&access_token=#{ACCESS_TOKEN}"
http = Net::HTTP.new(uri.host, uri.port)
http.use_ssl = true
http.verify_mode = OpenSSL::SSL::VERIFY_NONE
data = JSON.parse http.get(uri.request_uri).body
events = data['items']

data['items'].each do |event|
last_date = [last_date, event['creation_date'].to_i + 1].max
end
end
puts "#{data['quota_remaining']}/#{data['quota_max']} quota remaining"
sleep(40 + (data['backoff'] || 0).to_i) # add backoff time if any, just in case
}

rescue => e
$ERR = e p e end }  (Attribution to original author, code above is a modified version of it.) - Have an upvote: you'll be wanting to use the chatroom! – ChrisW Mar 16 '14 at 15:03 I can't believe I'm talking to a robot. This is awesome! – Mhmd Mar 16 '14 at 19:29 Ahem, I believe I licensed that under MIT, meaning you have to give attribution to me. ;) I'd prefer "The Supreme Overlordly Knob of the Door, Superior to Mankind in All Ways," but anything goes – Doorknob Apr 23 '14 at 13:14 Related: Chat bot feature requests on meta. – Simon Forsberg Sep 20 '15 at 19:40 ## 4 Answers 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(CHAT_ROOT + '/chats/join/favorite').forms.last.fkey
end

# 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

:room_number = 12723,
:site => 'codereview',
:email => ...,
loop {
begin

do
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."
}

-
Thanks for the review! In response to your comment in the code, take a look at the SE API here. – syb0rg Mar 16 '14 at 17:03

# Block syntax

This:

loop
{
...
}


Causes a syntax error in MRI 2.1. This would fix the syntax error:

loop {
...
}


However, the use of {...} is normally reserved for single-line blocks. Prefer:

loop do
..
end


# Methods

Use many more methods. It should be possible to figure out what the script does, in broad strokes, by looking only at its main method. Find lines of code that do one thing and put them in their own method. For example:

def login_to_se
login_form = $agent.get('https://openid.stackexchange.com/account/login').forms.first login_form.email = email login_form.password = password$agent.submit login_form, login_form.buttons.first
puts 'logged in with SE openid'
end

...



and so on. Your methods should, when possible, have these properties:

• The method does one thing
• The name says what it does
• All of the code in the method is at the same level of abstraction

You want code, at the higher levels such as the main loop, to look more like this:

loop do
continue_on_error do
loop do
copy_new_post_to_chat
wait
end
end
end


A method should read like a story. Abstract away--in methods, classes, etc--details that make the story hard to follow.

# Abstract out rescue, too

You may notice the call to continue_on_error above. It can be very useful to abstract out your rescue blocks, too. In this case, it gives us a method name that documents why we are doing the rescue:

def continue_on_error
yield
rescue => e
$ERR = e p e end  #$ERR

We can get rid of $ERR by having #continue_on_error say that we're restarting: def continue_on_error yield rescue => e puts e puts "Restarting" end  and in the main loop, instead of: puts$ERR ? "An unknown error occurred. Bot restarted." : "Bot initialized."


simply

puts "Initialized"


The script's log output will be just as clear.

-
Thanks for the review! I guess I was too used to my C-syntactical ways. :) – syb0rg Mar 16 '14 at 21:25
@syb0rg It seems to be accepted, in C, to write long methods. I've never understood the practice. – Wayne Conrad Mar 16 '14 at 21:28

Some low-level style issues:

• Although parentheses around parameter lists are optional, there is consensus that they should not be omitted.
• I don't see any consist pattern in your use the \$ sigil for variables. I suggest not using them at all.
• You use both Mechanize and raw Net::HTTP requests. I suggest using Mechanize for everything.
-
There is, I agree, pretty good consensus that parentheses should not be omitted in method declarations. What about method calls? – Wayne Conrad Mar 16 '14 at 19:01

1. You don't need to require rubygems as you are not using it at all. It is usually unnecessary. See here http://stackoverflow.com/questions/2711779/require-rubygems .
2. When you have many requires you can do this trick to group them into one line:

require 'rubygems'
require 'mechanize'
require 'json'
require 'net/http'


Into

%w{rubygems mechanize json net/http}.each{|gem| require gem}

-