Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have this script to pull data out of the Filepicker API internal. It's mostly reverse-engineering and the code seems to be ugly to me. How can this be improved?

yesterday = (Time.now - 1.day).to_i
start_date = "08/08/2013".to_time.to_i
url = URI.parse("https://developers.inkfilepicker.com/login/")
http = Net::HTTP.new(url.host, url.port)
http.use_ssl = (url.scheme == "https")
request = Net::HTTP::Post.new(url.request_uri)
request.set_form_data({"email" => "[email protected]", "password" => "123456"})
response = http.request(request)
cookie = response.response['set-cookie'].split('; ')[0]
uri = URI("https://developers.inkfilepicker.com/apps/XXX/stats/file?startdate=#{start_date}&enddate=#{yesterday}")
req = Net::HTTP::Get.new(uri.request_uri)
req['Cookie'] = cookie
res = Net::HTTP.start(uri.hostname, uri.port, :use_ssl => uri.scheme == 'https') {|http|
  http.request(req)
}
stats = JSON.parse res.body
stats = stats['data']['links.created']

In case of some of you are against reverse engineering, I asked for a 'real' API to pull this data and they suggested me to do this until they provide an API.

share|improve this question
    
no feedback...? –  tokland May 22 '14 at 10:31
    
Sorry! I've been following your best practice. I actually use the code with your suggestions! Thanks a lot! –  bl0b May 22 '14 at 17:44

1 Answer 1

up vote 2 down vote accepted

Looks pretty good. Some notes:

  • (Time.now - 1.day) -> 1.day.ago
  • "08/08/2013".to_time: I'd be in doubt "is that day/month or month/day?". An alternative to avoid problems with the parsing format: Time.new(2013, 8, 8)
  • Net::HTTP: IMO this module is a pain to use for common tasks. I prefer something higher-level like the rest-client gem. It does a lot of boring things for you.
  • response.response['set-cookie'].split('; ')[0]. I wouldn't rely on this space after the semi-colon. Safer: response.response['set-cookie'].split(';')[0].strip
  • Use do/end for multiline blocks.
  • JSON.parse res.body Be consistent, use parentheses like you did in the rest of the code.
  • stats = stats['data']['links.created']: don't reuse variables: links_created = stats['data']['links.created']
  • request and later req, response and later res: Use more declarative names.
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.