From 37dd436d756cf92e8db2fc77881f0fb83ce6a6a7 Mon Sep 17 00:00:00 2001 From: Jacob Evelyn Date: Mon, 29 May 2017 13:37:46 -0400 Subject: [PATCH] Improve display of favorites for ties Fixes #158. --- README.md | 4 +- friends.gemspec | 2 +- lib/friends/commands/list.rb | 26 +----- lib/friends/introvert.rb | 43 +++++++-- test/commands/list/favorite/friends_spec.rb | 86 +++++++++++++++++- test/commands/list/favorite/locations_spec.rb | 89 ++++++++++++++++++- 6 files changed, 212 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index 87aa6f6..e4e4fd6 100644 --- a/README.md +++ b/README.md @@ -487,7 +487,7 @@ $ friends list favorite friends Your favorite friends: 1. George Washington Carver (2 activities) 2. Grace Hopper (1) -3. Marie Curie (1) +3. Marie Curie (0) ``` You can specify a number of favorites to show: @@ -508,7 +508,7 @@ $ friends list favorite locations Your favorite locations: 1. Atlantis (2 activities) 2. Paris (1) -3. London (1) +3. London (0) ``` You can specify a number of favorites to show: diff --git a/friends.gemspec b/friends.gemspec index 520c99f..5d75b4a 100644 --- a/friends.gemspec +++ b/friends.gemspec @@ -29,7 +29,7 @@ Gem::Specification.new do |spec| spec.add_dependency "paint", "~> 1.0" spec.add_dependency "semverse", "~> 1.2" - spec.add_development_dependency "bundler", "~> 1.6" + spec.add_development_dependency "bundler", "~> 1.15" spec.add_development_dependency "coveralls", "~> 0.8" spec.add_development_dependency "minitest", "~> 5.5" spec.add_development_dependency "minitest-proveit", "~> 1.0" diff --git a/lib/friends/commands/list.rb b/lib/friends/commands/list.rb index 6907986..d774b7a 100644 --- a/lib/friends/commands/list.rb +++ b/lib/friends/commands/list.rb @@ -101,18 +101,7 @@ type: Integer list_favorite_friends.action do |_, options| - favorites = @introvert.list_favorite_friends(limit: options[:limit]) - - if options[:limit] == 1 - puts "Your best friend is #{favorites.first}" - else - puts "Your favorite friends:" - - num_str_size = favorites.size.to_s.size + 1 - favorites.each.with_index(1) do |name, rank| - puts "#{"#{rank}.".ljust(num_str_size)} #{name}" - end - end + @introvert.list_favorite_friends(limit: options[:limit]) end end @@ -125,18 +114,7 @@ type: Integer list_favorite_locations.action do |_, options| - favorites = @introvert.list_favorite_locations(limit: options[:limit]) - - if options[:limit] == 1 - puts "Your favorite location is #{favorites.first}" - else - puts "Your favorite locations:" - - num_str_size = favorites.size.to_s.size + 1 - favorites.each.with_index(1) do |name, rank| - puts "#{"#{rank}.".ljust(num_str_size)} #{name}" - end - end + @introvert.list_favorite_locations(limit: options[:limit]) end end end diff --git a/lib/friends/introvert.rb b/lib/friends/introvert.rb index 320d5d0..24a1c95 100644 --- a/lib/friends/introvert.rb +++ b/lib/friends/introvert.rb @@ -504,15 +504,42 @@ def list_favorite_things(type, limit:) -thing.n_activities end.take(limit) # Trim the list. - max_str_size = results.map(&:name).map(&:size).max - results.map.with_index(0) do |thing, index| - name = thing.name.ljust(max_str_size) - n = thing.n_activities - if index.zero? - label = n == 1 ? " activity" : " activities" + if results.size == 1 + favorite = results.first + puts "Your favorite #{type} is "\ + "#{favorite.name} "\ + "(#{favorite.n_activities} "\ + "#{favorite.n_activities == 1 ? 'activity' : 'activities'})" + else + puts "Your favorite #{type}s:" + + max_str_size = results.map(&:name).map(&:size).max + + grouped_results = results.group_by(&:n_activities) + + rank = 1 + first = true + data = grouped_results.each.with_object([]) do |(n_activities, things), arr| + things.each do |thing| + name = thing.name.ljust(max_str_size) + if first + label = n_activities == 1 ? " activity" : " activities" + first = false + end + str = "#{name} (#{n_activities}#{label})" + + arr << [rank, str] + end + rank += things.size + end + + # We need to use `data.last.first` instead of `rank` to determine the size + # of the numbering prefix because `rank` will simply be the size of all + # elements, which may be too large if the last element in the list is a tie. + num_str_size = data.last.first.to_s.size + 1 unless data.empty? + data.each do |ranking, str| + puts "#{"#{ranking}.".ljust(num_str_size)} #{str}" end - parenthetical = "(#{n}#{label})" - "#{name} #{parenthetical}" end end diff --git a/test/commands/list/favorite/friends_spec.rb b/test/commands/list/favorite/friends_spec.rb index 8949f37..4ab7957 100644 --- a/test/commands/list/favorite/friends_spec.rb +++ b/test/commands/list/favorite/friends_spec.rb @@ -45,6 +45,69 @@ OUTPUT end + describe "when friends are tied for the same number of activities" do + let(:content) do + <<-FILE +### Activities: +- 2017-01-01: Did something with **Friend A**. +- 2017-01-01: Did something with **Friend A**. +- 2017-01-01: Did something with **Friend B**. +- 2017-01-01: Did something with **Friend B**. +- 2017-01-01: Did something with **Friend C**. +- 2017-01-01: Did something with **Friend D**. +- 2017-01-01: Did something with **Friend E**. +- 2017-01-01: Did something with **Friend F**. +- 2017-01-01: Did something with **Friend G**. +- 2017-01-01: Did something with **Friend H**. +- 2017-01-01: Did something with **Friend I**. +- 2017-01-01: Did something with **Friend J**. + +### Friends: +- Friend A +- Friend B +- Friend C +- Friend D +- Friend E +- Friend F +- Friend G +- Friend H +- Friend I +- Friend J +FILE + end + + it "uses tied ranks" do + subject[:stderr].must_equal "" + subject[:status].must_equal 0 + + lines = subject[:stdout].split("\n") + lines[1].must_match(/1\. Friend (A|B)/) + lines[2].must_match(/1\. Friend (A|B)/) + lines[3].must_include "3. Friend" + end + + it "only uses the word 'activities' for the first item, even when a tie" do + subject[:stderr].must_equal "" + subject[:status].must_equal 0 + + lines = subject[:stdout].split("\n") + lines[1].must_include "activities" + lines[2].wont_include "activities" + end + + it "indents based on the highest rank number, not the number of friends" do + subject[:stderr].must_equal "" + subject[:status].must_equal 0 + + # Since there are 10 friends, a naive implementation would pad our output + # assuming the (numerically) highest rank is "10." but since the highest + # rank is a tie, we never display a double-digit rank, so we don't need to + # pad our output for double digits. + lines = subject[:stdout].split("\n") + lines.last.must_include "3. Friend" + end + end + describe "--limit" do subject { run_cmd("list favorite friends --limit #{limit}") } @@ -57,8 +120,27 @@ describe "when limit is 1" do let(:limit) { 1 } - it "outputs as a best friend" do - stdout_only "Your best friend is Grace Hopper (3 activities)" + + it "uses correct friend pluralization" do + stdout_only "Your favorite friend is Grace Hopper (3 activities)" + end + + describe "when favorite friend only has one activity" do + let(:content) do + <<-FILE +### Activities: +- 2017-01-01: Did some math with **Grace Hopper**. + +### Friends: +- George Washington Carver +- Grace Hopper (a.k.a. The Admiral a.k.a. Amazing Grace) [Paris] @navy @science +- Marie Curie [Atlantis] @science +FILE + end + + it "uses correct activity pluralization" do + stdout_only "Your favorite friend is Grace Hopper (1 activity)" + end end end diff --git a/test/commands/list/favorite/locations_spec.rb b/test/commands/list/favorite/locations_spec.rb index 40fa24c..85311a1 100644 --- a/test/commands/list/favorite/locations_spec.rb +++ b/test/commands/list/favorite/locations_spec.rb @@ -50,6 +50,69 @@ OUTPUT end + describe "when locations are tied for the same number of activities" do + let(:content) do + <<-FILE +### Activities: +- 2017-01-01: Did something in _Location A_. +- 2017-01-01: Did something in _Location A_. +- 2017-01-01: Did something in _Location B_. +- 2017-01-01: Did something in _Location B_. +- 2017-01-01: Did something in _Location C_. +- 2017-01-01: Did something in _Location D_. +- 2017-01-01: Did something in _Location E_. +- 2017-01-01: Did something in _Location F_. +- 2017-01-01: Did something in _Location G_. +- 2017-01-01: Did something in _Location H_. +- 2017-01-01: Did something in _Location I_. +- 2017-01-01: Did something in _Location J_. + +### Locations: +- Location A +- Location B +- Location C +- Location D +- Location E +- Location F +- Location G +- Location H +- Location I +- Location J +FILE + end + + it "uses tied ranks" do + subject[:stderr].must_equal "" + subject[:status].must_equal 0 + + lines = subject[:stdout].split("\n") + lines[1].must_match(/1\. Location (A|B)/) + lines[2].must_match(/1\. Location (A|B)/) + lines[3].must_include "3. Location" + end + + it "only uses the word 'activities' for the first item, even when a tie" do + subject[:stderr].must_equal "" + subject[:status].must_equal 0 + + lines = subject[:stdout].split("\n") + lines[1].must_include "activities" + lines[2].wont_include "activities" + end + + it "indents based on the highest rank number, not the number of locations" do + subject[:stderr].must_equal "" + subject[:status].must_equal 0 + + # Since there are 10 friends, a naive implementation would pad our output + # assuming the (numerically) highest rank is "10." but since the highest + # rank is a tie, we never display a double-digit rank, so we don't need to + # pad our output for double digits. + lines = subject[:stdout].split("\n") + lines.last.must_include "3. Location" + end + end + describe "--limit" do subject { run_cmd("list favorite locations --limit #{limit}") } @@ -62,9 +125,33 @@ describe "when limit is 1" do let(:limit) { 1 } - it "outputs as a favorite location" do + + it "uses correct location pluralization" do stdout_only "Your favorite location is Marie's Diner (2 activities)" end + + describe "when favorite location only has one activity" do + let(:content) do + <<-FILE +### Activities: +- 2017-01-01: Did some math with **Grace Hopper** at _Marie's Diner_. + +### Friends: +- George Washington Carver +- Grace Hopper (a.k.a. The Admiral a.k.a. Amazing Grace) [Paris] @navy @science +- Marie Curie [Atlantis] @science + +### Locations: +- Atlantis +- Marie's Diner +- Paris +FILE + end + + it "uses correct activity pluralization" do + stdout_only "Your favorite location is Marie's Diner (1 activity)" + end + end end describe "when limit is greater than 1" do