Ruby command-line app that outputs from JSON file

Posted on

Problem

I made this application that runs fine, but I feel the code is sloppy as there are many if statements. I’m a beginner so I’m not sure how to improve it, at least not without messing everything up. Any suggestions?

#!/usr/bin/ruby

require 'thor'
require 'json'

class ListTrends < Thor

  desc "list_trends json", "list out trends from JSON file"
  option :api_key, :aliases => "--api-key", :type => :string, :required => true
  option :format, :type => :string, :desc => "one line format"
  option :no_country_code, :aliases => "--no-country-code", :desc => "remove country code", :type => :boolean
  def list_trends(keyword=nil)

    json = File.read('trends_available.json')
    trends_hash = JSON.parse(json)
    re = /^(?=.*[a-zA-Z])(?=.*[0-9]).{8,}$/
    keyword = keyword.to_s.downcase

    if re.match(options[:api_key])

      trends_hash.each do |trend|
        if trend["country"].downcase.include?(keyword)
          if options[:format]
            output = trend.values[0..2]
            output.delete_at(1) if options[:no_country_code]
            puts output.join(" ")
          else
            # Complete output
            trend.each do |k, v|
              if v.is_a?(Hash)
                v.each do |i, j| puts "Trend location type #{i}: #{j}" end
              else
                puts "Trend #{k}: #{v}"
              end
            end # trend.each do
            puts ""
          end # if options[:format]
        end # if trend["country"]
      end # trends_hash.each

    else
      puts "Invalid API Key, operation abort..."
    end # if re.match

  end # list_trends
end

ListTrends.start(ARGV)

Solution

The way to deal with nested if’s is generally to move code into methods. Another solutions is to use an ‘early return’. (if !valid b; return instead of if valid then a else b)

I would write this something like:

#!/usr/bin/ruby

require 'thor'
require 'json'

class ListTrends < Thor

  VALID_API_KEY_RE = /^(?=.*[a-zA-Z])(?=.*[0-9]).{8,}$/

  desc   'list_trends json', 'list out trends from JSON file'
  option :api_key, aliases: '--api-key', type: :string, required: true
  option :format, type: :string, desc: 'one line format'
  option :no_country_code, aliases: '--no-country-code', desc: 'remove country code', type :boolean

  def list_trends(keyword=nil)

    # Check your options before reading the file     
    if !VALID_API_KEY_RE.match(options[:api_key])
      puts "Invalid API Key, operation abort..."
      exit(255)
    end

    json        = File.read('trends_available.json')
    trends_hash = JSON.parse(json)
    keyword     = keyword.to_s.downcase

    trends_hash.each do |trend|
      process_trend(trend)
    end
  end

  private 

  def process_trend(trend)
    return unless trend["country"].downcase.include?(keyword)

    if options[:format]
      output_formatted_trend(trend)
    else
      output_complete_trend(trend)
    end
  end

  def output_formatted_trend(trend)
    output = trend.values[0..2]
    output.delete_at(1) if options[:no_country_code]
    puts output.join(' ')
  end  

  def output_complete_trend(trend)
    trend.each do |k, v|
      if v.is_a?(Hash)
        v.each do |i, j| puts "Trend location type #{i}: #{j}" end
      else
        puts "Trend #{k}: #{v}"
      end
      puts ''
    end    
  end

end

ListTrends.start(ARGV)

Another advantage being that most people can only understand a few lines of code at a time, short methods also make it easier to keep track of variable scope and giving them meaningful names makes it easier to understand code without needing comments.

Leave a Reply

Your email address will not be published. Required fields are marked *