From 0d31a28e91220b9fcc2487a4525b0a91da462c75 Mon Sep 17 00:00:00 2001 From: Andrew Kane Date: Thu, 6 Oct 2016 23:09:16 -0700 Subject: [PATCH] Use keyword arguments --- CHANGELOG.md | 4 ++++ lib/hightop.rb | 16 ++++++---------- test/hightop_test.rb | 6 ++++++ 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d654f1..c2e5413 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.2.0 [unreleased] + +- Use keyword arguments + ## 0.1.4 - Added `distinct` option to replace `uniq` diff --git a/lib/hightop.rb b/lib/hightop.rb index 52ab5f5..4593cb0 100644 --- a/lib/hightop.rb +++ b/lib/hightop.rb @@ -2,27 +2,23 @@ require "active_record" module Hightop - def top(column, limit = nil, options = {}) - if limit.is_a?(Hash) - options = limit - limit = nil - end - - distinct = options[:distinct] || options[:uniq] + def top(column, limit = nil, distinct: nil, uniq: nil, min: nil, nil: nil) + distinct ||= uniq order_str = column.is_a?(Array) ? column.map(&:to_s).join(", ") : column relation = group(column).order("count_#{distinct || 'all'} DESC, #{order_str}") if limit relation = relation.limit(limit) end - unless options[:nil] + # terribly named option + unless binding.local_variable_get(:nil) (column.is_a?(Array) ? column : [column]).each do |c| relation = relation.where("#{c} IS NOT NULL") end end - if options[:min] - relation = relation.having("COUNT(#{distinct ? "DISTINCT #{distinct}" : '*'}) >= #{options[:min].to_i}") + if min + relation = relation.having("COUNT(#{distinct ? "DISTINCT #{distinct}" : '*'}) >= #{min.to_i}") end if distinct diff --git a/test/hightop_test.rb b/test/hightop_test.rb index 6ec2549..5ffe371 100644 --- a/test/hightop_test.rb +++ b/test/hightop_test.rb @@ -101,6 +101,12 @@ def test_min_distinct assert_equal expected, Visit.top(:city, min: 2, distinct: :user_id) end + def test_bad_argument + assert_raises(ArgumentError) do + Visit.top(:city, boom: true) + end + end + def create_city(city, count = 1) create({city: city}, count) end