Skip to content

Conversation

@shirley1chu
Copy link

slack.rb

Congratulations! You're submitting your assignment!

You and your partner should collaborate on the answers to these questions.

Comprehension Questions

Question Answer
How did you go about exploring the Slack API? Did you learn anything that would be useful for your next project involving an API? We used binding.pry to look at the result of the API calls and to discover how the result data was stored in the JSON file. We were very surprised and frustrated by the lack of differentiation in the Slack API's response code (they're all 200 OK!) but it was helpful to use binding.pry to discover that "ok" could be true or false. This was a good lesson to take to our next API project - don't trust the response code!
Give a short summary of the request/response cycle. Where does your program fit into that scheme? The request/response cycle is when a client sends a request and receives a response from a server. The response comes back from the server in a variety of formats depending on the request protocol. Our program fits into that scheme as the client making the request. The command line program is a client and Slack API's server is the server.
How does your program check for and handle errors when using the Slack API? It checks the response code (ha! this is useless here) and the value of "ok" for the response. The program raises SlackApiErrors everywhere but the driver code. Certain methods return nil for bad input. In main.rb, we don't want to break the program when a user gives invalid input so instead we just keep the user in the loop until they give valid input or quit.
Did you need to make any changes to the design work we did in class? If so, what were they? We made one small changes of passing an argument in Workspace#send_message. We thought this was the most logical way for the message to get passed to the selected recipient.
Did you use any of the inheritance idioms we've talked about in class? How? Recipient served as an abstract class with template methods. We used polymorphism to send messages to both users and channels. Users and channels had methods with the same name so even though they had different types of attributes, they could respond to the same types of messages.
How does VCR aid in testing a program that uses an API? It records an API call that the program makes once so that when a method is called in a test, it isn't repeatedly pinging an API server.

@droberts-sea
Copy link

slack.rb

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene (no slack tokens) yes
Comprehension questions yes
Functionality
List users/channels yes
Select user/channel yes
Show details yes
Send message yes
Program runs without crashing yes
Implementation
API errors are handled appropriately yes
Inheritance model matches in-class activity yes
Inheritance idioms (abstract class, template methods, polymorphism) used appropriately yes
Methods are used to break work down into simpler tasks yes
Class and instance methods are used appropriately yes
Overall Excellent job overall. This code is well-written and well-tested. It is clear to me that the learning goals around understanding the request/response cycle, consuming and API, and implementing a design using inheritance from scratch were all met. I've left a few inline comments for you to review below, but in general I'm quite impressed by what I see here. Keep up the hard work!


# private

def self.list

Choose a reason for hiding this comment

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

Why is private commented out?

module Slack
class Workspace
attr_reader :users, :channels
attr_accessor :selected

Choose a reason for hiding this comment

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

I think an attr_reader might be the right call for selected as well, since it's only changing from within this class.

def select_channel(value)
@channels.each do |channel|
if channel.name == value
@selected = channel

Choose a reason for hiding this comment

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

You have a lot of repeated code between this method and select_user above. Is there a way you could DRY this up?

id = "cat"
selected_user = @workspace.select_user(id)

expect(selected_user).must_be_nil

Choose a reason for hiding this comment

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

What happens to a previously selected user?


it "returns nil if no recipient" do
expect(@workspace.show_details).must_be_nil
end

Choose a reason for hiding this comment

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

You've done a great job throughout this project of identifying and testing failure cases.

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