Sockets - Sarah/Riyo #14
Conversation
… CLI program options
lib/slack.rb
Outdated
| puts "Sorry, no user with that information" if selected == nil | ||
| when "select channel" | ||
| puts "Please enter a channel name channel ID" | ||
| input = get_user_input |
There was a problem hiding this comment.
This crashes because get_user_input isn't passed message_to_user. You should either make the argument optional (say by adding a default value for the argument) or pass a message in here like you did above.
There was a problem hiding this comment.
It's really easy to miss things like this in code that's difficult to unit test. Once we get to Rails it will be easier to write unit tests on your driver code.
| def self.get(path_url) | ||
| query_parameters = { token: TOKEN } | ||
| response = HTTParty.get("#{BASE_URL}#{path_url}", query: query_parameters) | ||
| return response |
There was a problem hiding this comment.
I would have liked to see check_response_code called inside of self.get instead of outside. If you have to validate separately then it's easy to forget.
| require_relative "../lib/workspace.rb" | ||
|
|
||
| def get_user_options_input(options) | ||
| input = gets.chomp.to_s.strip |
There was a problem hiding this comment.
Minor issue: .to_s is redundant here (gets already gives you a string).
|
|
||
| def self.check_response_code(response) | ||
| unless response.code == 200 && response.parsed_response["ok"] | ||
| raise SlackApiError, "Error when posting #{message} to #{id}, error: #{response.parsed_response["error"]}" |
There was a problem hiding this comment.
This references message and id but those methods aren't defined here and the variables aren't passed in.
I suspect message should be a parameter here and id should be an attr_reader and required by initialize (since you want all Recipients to have an id).
| end | ||
|
|
||
| def select_user(input) | ||
| found_user = @users.find do |user| |
| found_user = @users.find do |user| | ||
| input == user.id || input == user.name | ||
| end | ||
| @selected = found_user |
There was a problem hiding this comment.
If you aren't going to do additional validation you can just directly assign @selected to found_user.
| end | ||
|
|
||
| def send_message(message) | ||
| return false if !selected |
There was a problem hiding this comment.
I like that you checked for selected here.
Remember, you can use unless selected instead of if !selected to clarify things a touch (though that's mostly a matter of personal preference).
| return false if !selected | |
| return false unless selected |
| end | ||
|
|
||
| it "Initializes with selected equal to nil" do | ||
| assert_nil @workspace.selected |
There was a problem hiding this comment.
This is a little tricky to Google for but, you can use an expect style test with must_be_nil.
| assert_nil @workspace.selected | |
| expect(@workspace.selected).must_be_nil |
slack.rb
Congratulations! You're submitting your assignment!
You and your partner should collaborate on the answers to these questions.
Comprehension Questions