# Command-line search for web-scraped TV Guide listings

This is a project that I am looking to clean up. The code is fully functioning and it has already been turned in, so I figured I can let Code Review help me. The git repository can be found here: https://github.com/TylerP33/sports_today

def user_input

input = nil
input = gets.strip.downcase

case input

when  "football"
info = SportsToday::Schedule.football
show_info(info)
post_search_options

when "baseball"
info = SportsToday::Schedule.baseball
show_info(info)
post_search_options

show_info(info)
post_search_options

show_info(info)
post_search_options

…


The code above calls these methods in the SportsToday::Schedule class, which search for specific strings in attributes (derived from scraped HTML):

def self.select_by(sport)
all.select do |object|
ATTRIBUTES.any? { |attr| object.public_send(attr).include?(sport) }
end
end

def self.football
select_by("Football")
end

def self.baseball
select_by("Baseball")
end

end

select_by("NBA")
end

…


And then, the results are displayed using this:

def show_info(info)
info.collect {|info|
puts""
puts""
puts "\nEvent: #{info.sport}"
puts "\nTime: #{info.time}"
puts "\nDescription: #{info.description}"
puts "\nMatchup: #{info.matchup}"
puts ""
puts ""
}
end


I have some nasty code repetition going on — 10 similar methods in my CLI and Schedule classes — and I have no idea how to get around it. There HAS to be a way for me to make some type of container for keywords, and be able to access them through my select_by method.

• A couple of quick things: Indentation (2 spaces), use do/end for multiline blocks, and puts "\n"*2 is the same as puts ""; puts "". – thesecretmaster Feb 22 '18 at 20:01
• Pretty nifty trick on the puts "\n"*2! Also, I never knew about that convention regarding multiline blocks. I will definitely use that. Thanks for the advice! – TylerP3358 Feb 22 '18 at 23:15
• No problem! I'll try to write a full review if I have time. – thesecretmaster Feb 22 '18 at 23:53
• That would be nice... I am sure some abstraction could definitely be used. The more I continue to learn, the more the code is starting to look more and more elementary. I guess that is how learning is suppose to go :D. But really, I would love to figure out how to improve the select_by functionality of it all. – TylerP3358 Feb 23 '18 at 3:51

# Refactoring #1

The first thing I noticed is the repetition in user_input. You're using the case statement as flow control--it's no different than a series of if/else statements. Instead, use the case statement to set your info variable and move the other statements out:

info = case input
when "football"           then SportsToday::Schedule.football
when "baseball"           then SportsToday::Schedule.baseball
end

show_info(info)
post_search_options


# Refactoring #2

After the first refactoring, it becomes clear that the case statement is simply translating a String to a method name (which gets translated back to a String in the method itself). This is inefficient. It might be tempting to generate the method name:

SportsToday::Schedule.call(input.tr(" ","_"))


but all that does is mask the underlying problem: you have a method for every possible search string.

Instead, consider changing the interface of the class. Instead of one method per search string, use one for all:

#SportsToday::Schedule

def self.sport(sportname)
select_by(sportname)
end


Not only does this simplify your SportsToday::Schedule class, it simplifies your CLI:

info = SportsToday::Schedule.sport(input)
show_info(info)
post_search_options


Note that I "cheated" a little on this refactoring, as the interface changes slightly. The user needs to enter the case of the sport name differently. However, it's not much different than before, as an exact match was required in your original code too. If the intent is to make it accommodate user input, you can adjust for user input sloppiness in your Schedule class. I wouldn't do it in the CLI, because it requires knowledge of the Schedule's underlying data format and that should be kept encapsulated in your Schedule class.

[I don't have time to add more refactorings right now, I'll try to come back to this and add more.]

• Mark, thanks a ton man! Sorry for the late response! I have been busy and haven't checked back here in sometime. These are brilliant ideas and I will definitely make use of them when I get a chance. Thanks for taking the time and writing all of that! – TylerP3358 Apr 11 '18 at 20:03