Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Panda and Tiger Submission #29

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Panda and Tiger Submission #29

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e7f63e2
add 2 more shows to the seed file, update show output format
jperezish Jan 15, 2014
2a29cff
add CLI to ask for day of week, app returns shows that match the prov…
jperezish Jan 16, 2014
2268c9f
create review table, create ui to access a movie review
jperezish Jan 22, 2014
f3bd867
update to use DB where query
jperezish Jan 23, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions db/seed.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
# Cleaning Out
Network.delete_all
Show.delete_all
amc = Network.create(name: "AMC")
nbc = Network.create(name: "NBC")
amc = Network.create(name: "AMC")
nbc = Network.create(name: "NBC")
bravo = Network.create(name: "Bravo")
Show.create(name: "Mad Men", day_of_week: "Sunday", hour_of_day: 22, network: amc)
Show.create(name: "Community", day_of_week: "Thursday", hour_of_day: 20, network: nbc)
Show.create(name: "Top Chef", day_of_week: "Wednesday", hour_of_day: 19, network: bravo)
Show.create(name: "Walking Dead", day_of_week: "Sunday", hour_of_day: 20, network: amc)
2 changes: 1 addition & 1 deletion models/show.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ class Show < ActiveRecord::Base
validates_presence_of :name

def to_s
"#{name} airs at #{hour_of_day}:#{day_of_week}:00 on #{network} "
"#{name} airs at #{day_of_week}, #{hour_of_day}:00 on #{network}."
end
end
17 changes: 12 additions & 5 deletions watchman.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,18 @@
Dir.glob('./models/*').each { |r| require r}
require "./db/seed"

puts "***********************"
puts "* Welcome to WATCHMAN *"
puts "***********************"
puts "There are #{Show.count} in the database"
puts ""
puts "What day would you like to watch shows?"
print "> "

day_of_week = gets.chomp.downcase

Network.all.each do |network|
puts "Shows airing on #{network}"
network.shows.each do |show|
puts show
end
end
network.shows.each do |show|
puts show if show.day_of_week.downcase == day_of_week
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent use of network.shows :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have an array where you want to do something to each element, I'd rather you limit your collection first, rather than loop over everything and selectively do something. It will lead to blocks that you can reuse and extract to methods, and it reads your intent easier.

network.shows.each do |show|
  puts show if show.day_of_week.downcase == day_of_week
end

vs

network.shows.select{ |show| show.day_of_week.downcase == day_of_week }.each do |show|
  puts show
end

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...limit your collection first, rather than loop over everything and selectively do something. It will lead to blocks that you can reuse and extract to methods, and it reads your intent easier.

That makes a ton of sense. Thanks!

Would you recommend assigning network.shows.select{ |show| show.day_of_week.downcase == day_of_week } to a variable for readability? That line is 87 chars.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that would work. Or you can have a method:

def for_day_of_week(shows, day)
  shows.select{ |show| show.day.downcase == day }
end

for_day_of_week(network.shows, day_of_week).each do |show|
  puts show
end

And, we could get even fancier with the last. The naming "for_day_of_week" could be "each_day_of_week", implying we give it a block (though it is fairly advanced, and you'd only do this if you needed to re-use each_day_of_week)

def each_day_of_week(shows, day)
  shows.select{ |show| show.day.downcase == day }.each do |show|
    yield show
  end
end

each_day_of_week(network.shows, day_of_week) do |show|
  puts show
end

end
end