Skip to content

Commit

Permalink
Improve display of favorites for ties
Browse files Browse the repository at this point in the history
Fixes #158.
  • Loading branch information
JacobEvelyn committed May 29, 2017
1 parent 8910b84 commit 37dd436
Show file tree
Hide file tree
Showing 6 changed files with 212 additions and 38 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion friends.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
26 changes: 2 additions & 24 deletions lib/friends/commands/list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down
43 changes: 35 additions & 8 deletions lib/friends/introvert.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
86 changes: 84 additions & 2 deletions test/commands/list/favorite/friends_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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}") }

Expand All @@ -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

Expand Down
89 changes: 88 additions & 1 deletion test/commands/list/favorite/locations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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}") }

Expand All @@ -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
Expand Down

0 comments on commit 37dd436

Please sign in to comment.