Skip to content

Panda and Tiger Submission - #29

Open
jperezish wants to merge 4 commits into
RubyoffRails:masterfrom
jperezish:master
Open

Panda and Tiger Submission#29
jperezish wants to merge 4 commits into
RubyoffRails:masterfrom
jperezish:master

Conversation

@jperezish

Copy link
Copy Markdown

No description provided.

Comment threadwatchman.rb Outdated

Copy link
Copy Markdown
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
Copy Markdown
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.eachdo |show|
putsshowifshow.day_of_week.downcase == day_of_weekend

vs

network.shows.select{ |show| show.day_of_week.downcase == day_of_week}.eachdo |show|
putsshowend

Copy link
Copy Markdown
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
Copy Markdown
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:

deffor_day_of_week(shows,day)shows.select{ |show| show.day.downcase == day}endfor_day_of_week(network.shows,day_of_week).eachdo |show|
putsshowend

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)

defeach_day_of_week(shows,day)shows.select{ |show| show.day.downcase == day}.eachdo |show|
yieldshowendendeach_day_of_week(network.shows,day_of_week)do |show|
putsshowend

@jperezish

Copy link
Copy Markdown
Author

@jwo added my Eagle level submission. Thanks!

Comment threadwatchman.rb Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Dont think you need to keep this, right?

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@jperezish@jwo