Skip to content

Week 3 pull request - Panda and Tiger Levels #13

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Week 3 pull request - Panda and Tiger Levels #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions db/seed.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,9 @@
Show.delete_all
amc = Network.create(name: "AMC")
nbc = Network.create(name: "NBC")
sho = Network.create(name: "Showtime")
fox = Network.create(name: "Fox")
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: "Homeland", day_of_week: "Sunday", hour_of_day: 22, network: sho)
Show.create(name: "Fringe", day_of_week: "Friday", hour_of_day: 21, network: fox)
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 on #{day_of_week} at #{hour_of_day}:00 on #{network} "
end
end
25 changes: 23 additions & 2 deletions watchman.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,36 @@
require 'rubygems'
require 'bundler/setup'
require 'date'

require "./db/setup"
Dir.glob('./models/*').each { |r| require r}
require "./db/seed"

puts "There are #{Show.count} in the database"

def ask_user_for_valid_day_of_week
user_date = gets.chomp.capitalize
days_of_the_week = Date::DAYNAMES
count = 0
max_number_of_tries = 2
while days_of_the_week.index(user_date).nil? && count < max_number_of_tries
puts "That is not a valid day of the week. Please try again."
Copy link
Member

Choose a reason for hiding this comment

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

You could probably pull this out into two different methods... maybe something like:

def retry_find_date(times)
  retry_attempts = 0
  begin
    retry_attempts += 1
    return find_date
  rescue InvalidDateError
    if retry_attempts < times
      puts "That is not a valid day of the week. Please try again."
      retry
    else
      puts "No valid date entered, giving shows for today (#{user_date})"
      return days_of_the_week[Time.new.wday]
    end
  end
end

class InvalidDateError < StandardError; end

def find_date
  Date::DAYNAMES.fetch(gets.chomp.capitalize) { raise InvalidDateError.new }
end

Then you'd use it in subjectively cleaner way:

date = retry_find_date(3)
Network.all###

user_date = gets.chomp.capitalize
count += 1
end
if count == max_number_of_tries
user_date = days_of_the_week[Time.new.wday]
puts "No valid date entered, giving shows for today (#{user_date})"
end
user_date
end

puts "What day of the week do you want to watch shows?"
day_of_interest = ask_user_for_valid_day_of_week

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

Choose a reason for hiding this comment

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

Rather than doing an if statement inside of a loop, I recommend you get an array of items that are valid first, and then loop over those items to puts show. Something like:

valid_shows = Network.all.select{|s| s.day_of_week == day_of_interest}
valid_shows.each do |show|
  puts show
end

Which could be combined if you don't need to access the valid_shows anymore:

Network.all.select{|s| s.day_of_week == day_of_interest}.each do |show|
  puts show
end

end
end
end