Refactor Ruby code from Hacker News API. Trouble parsing the JSON data

I made a command line Hacker News reader in Ruby that pulls the latest 10 stories from HN and allows the user to cycle through them or open them with Launchy. This was to play around with Ruby and APIs.

The data that I pulled from HN was messy, like an array of hashes with arrays and hashes inside. I feel like my code is messy, and I am unsure of how it can be made cleaner.

Any advice? I'm still a begninner.

news.rb --

class News
  def initialize title, url, author
    @title = title
    @url = url
    @author = author
  end

  # Format display of each story.
  def to_s
    "\n\tTitle:  #{@title}\n
    \tAuthor: #{@author}\n
    \tUrl: #{@url}\n\n"
  end

  # Insert current object's url into a variable accessible by Launchy
  def get_url
    @url
  end

end

# Show the news story, and ask if user wants open link or move on.
def show_story
    puts @stories[@counter]
    @link = @stories[@counter].get_url
    puts "To view this story in your browser, type OPEN\n
    To see the next story, type NEXT"
    @start = gets.chomp.downcase
    shuffle
  end

# Determine if user wants to open link or move on.
def shuffle
  if @start == "open"
    Launchy.open(@link)
    next_story
  elsif @start == "next"
    @counter += 1
    show_story
  else
    puts "Sorry, you didn't type a correct command.\n
    To view this story in your browser, type OPEN\n
    To see the next story, type NEXT"
    @start = gets.chomp.downcase
    shuffle
  end
end

# After opening link, ask if user still wants to see next story
def next_story
  puts "Would you still like to see the next story? Type NEXT"
  @start = gets.chomp.downcase
  shuffle
end

runner.rb --

```
require 'json'
require 'rest-client'
require 'launchy'
require_relative 'news.rb'

# Pull the latest/top 10 stories from Hacker News
data = RestClient.get('http://api.thriftdb.com/api.hnsearch.com/items/_search?limit=10&sortby=product(points,pow(2,div(div(ms(create_ts,NOW),3600000),72)))%20desc&pretty_print=true')

# Parse us

Redundant operations

After reading the data from the web service, you go over it a number of times, which seems redundant:

results = parsed.reject { |key, value| key != "results"}

Why is that important? You go over the whole JSON, and discard everything - but you gain nothing. Simply use only what you need, no need to actively remove it.

pull_items = []
pull_items = results["results"].each do |items|
  pull_items << items
end

This is simply a very convoluted way of saying pull_items = results['results']...

pull_info = []
pull_items.each do |x|
  pull_info << x["item"]
end

Ruby's idiomatic way of saying the same thing is pull_info = pull_items.map { |x| x['item'] }

@stories = pull_info.map do |s|
  News.new s["title"], s["url"], s["username"]
end

If this is what you actually wanted, you don't really need all the temporary artifacts in the middle - simply do it in one swoop:

@stories = results['results'].map do |r| 
  item = r['item']
  News.new item['title'], item['url'], item['username']
end

Naming Conventions

It is conventional in ruby to name getters by the name of the member, without the get_XXX. For that convention, there is even a shortcut - attr_reader. So, instead of writing def get_url you should do:

class News
  attr_reader :url
  ...
end

@link = @stories[@counter].url

and it will just work...

Code Encapsulation

A ruby file should contain a single class. It should not contain any floating methods.

Actually, there should not be any floating methods. Create a different class, or a module, which will contain the show_story, shuffle, and next_story. This will also force you to name the module, and make it more obvious what is its role in your application.

StackExchange Code Review Q#46258, answer score: 3