Skip to content

Conversation

@Meshed
Copy link

@Meshed Meshed commented Nov 28, 2012

I still need to refactor my test, but i hope to go over that in the office hour if you don't already have a plan.

lib/api.rb Outdated
Copy link
Member

Choose a reason for hiding this comment

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

I don't like class variables in most cases... This will prevent you from going multi-threaded in the future (as each thread will overwrite this variable)

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, that is a left over artifact from before I implemented the Search_History class.

@jwo
Copy link
Member

jwo commented Nov 28, 2012

Good stuff here!

I especially love the way the search_history class ended up looking... Looks great (small methods, readable, clean).

One minor note --- I found that if I had some valid searches, and then mispelled a classic (Billy Maddison), it would use 0 as a score and drop my movie score

Add a movie you really like
Fletch
Found: Fletch. Score: 75
Average movie score: 75.
Average movie year: 1982.
You are getting madder
Search Again (Y/N)
Y
Add a movie you really like
Billy Maddison
Found: NOTHINGFOUNDHERE. Score: 0
Average movie score: 50.
Average movie year: 1321.
Search Again (Y/N)
Y

@Meshed
Copy link
Author

Meshed commented Nov 28, 2012

Thank you for your great suggestions. I will add a test for the NOTHINGFOUNDHERE score bug.

Sign up for free to 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