Branches - Farah && Steph #11
Conversation
slack.rbWhat We're Looking For
Great work on this project, Farah and Steph! Your code is logical and clean, and your tests are well-constructed. You two did a great job getting the API functionality going and wrapping in tests, VCR, and design. I have a few comments about the submission:
Any other comment I have a minor one about handling API errors: we didn't go into handling API errors in depth during class, but I'd encourage you two to start thinking about: what happens if my response comes back with a status code that's not Again good work on this overall! |
| @@ -0,0 +1,57 @@ | |||
| .env | |||
There was a problem hiding this comment.
we actually don't need a .gitignore file in the lib folder... just one in the project root directory!
| def initialize(slack_id, name, topic, member_count) | ||
| super(slack_id, name) | ||
| @topic = topic | ||
| @member_count = member_count |
There was a problem hiding this comment.
Do @topic or @member_count get used? Could we delete these?
| super(slack_id, name) | ||
| @topic = topic | ||
| @member_count = member_count | ||
| @@channels_list << self |
There was a problem hiding this comment.
I think that it's very clever to use the class variable @@channels_list and to shovel in self! However, does this variable ever get used beyond this? Do we need it?
| @@channels_list << self | ||
| end | ||
|
|
||
| def self.list |
| puts"Welcome to the Ada Slack CLI!" | ||
| puts"Select an option by number:" | ||
| options_array=["List Channels","List Users","Select User","Select Channel","Quit"] | ||
| options_array.each_with_index{ |channel, index| |
There was a problem hiding this comment.
Nitpick: the syntax for this line means that every element in options_array when we iterate through it will be named channel... but options_array doesn't contain channels, it contains options! Would it make sense if this variable were named option instead of channel?
|
|
||
| while user_answer != 5 | ||
| if user_answer == 1 | ||
| Channel.printed_channels_list |
| end | ||
|
|
||
| puts "Select an option by number:" | ||
| options_array.each_with_index { |channel,index| |
There was a problem hiding this comment.
Nitpick: same as above; instead of channel, is there a better variable name?
| channels_array = Channel.printed_channels_list | ||
| channels_array.each do |channel| | ||
| if channel["name"] == "general" | ||
| assert (channel), "Expected true" |
There was a problem hiding this comment.
To be honest, I'm just not sure what this line of code does-- what does it test?
| end | ||
| end | ||
|
|
||
| it "should return an array of channels" do |
There was a problem hiding this comment.
Looking at your implementation, the Channel.printed_channels_list method DOES NOT return an array of channels, and it returns an array of hashes! Your test name and test contents should reflect this
| end | ||
|
|
||
| describe "self.select_channel_details method" do | ||
| it "should return true four 'general' details" do |
There was a problem hiding this comment.
this is a great test! Good detail! Keep doing things like this when it's appropriate!
| end | ||
| end | ||
|
|
||
| it "should return 'Not valid' statement if the Channel does not exist" do |
3 participants
slack.rb
Congratulations! You're submitting your assignment!
You and your partner should collaborate on the answers to these questions.
Comprehension Questions