Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ruby/Resistor Colors: Add Mentor Notes #853

Merged
merged 2 commits into from
Mar 10, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions tracks/ruby/exercises/resistor-colors/mentoring.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
Resistor Colors is a side exercise, unlocked by Hello World

### New Concepts
Array, Array#index, `map`, Constant, chaining methods
Copy link
Member

Choose a reason for hiding this comment

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

Add blank line between header and content, following default markdownlint


### Minimal solution for approval
```ruby

Copy link
Member

Choose a reason for hiding this comment

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

I suggest removing the whitespace above class, but adding an empty line below the header, following default markdownlint.

class ResistorColors

COLOR_CODES = ["black", "brown", "red", "orange", "yellow", "green", "blue", "violet", "grey", "white"]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
COLOR_CODES = ["black", "brown", "red", "orange", "yellow", "green", "blue", "violet", "grey", "white"]
COLOR_CODES = %w[black brown red orange yellow green blue violet grey white].freeze

Default style via rubocop WordArray and rubocop MutableConstant.

Add below this solution that:

  • "Alternatives include writing out the array using brackets"; the talking points already mention this.
  • freeze should only be a talking point but not a reason to disapprove


def self.value(colors)
colors.map { |color| COLOR_CODES.index(color) }.join.to_i
end

end

Copy link
Member

Choose a reason for hiding this comment

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

Remove whitespace between final end and end of code block.

```

Copy link
Member

Choose a reason for hiding this comment

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

Add note about acceptable variations include using a module, class << self, module_function and the likes. We don't want mentors to comment on any of that, seeing it's such an early exercise!

### General
This is the very first Array exercise, and the first loop. Introducing `map` is key in this exercise.
Copy link
Member

@pgaspar pgaspar Mar 10, 2019

Choose a reason for hiding this comment

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

This is the very first Array exercise

Isn't this set up as a bonus exercise? (reference)


### Talking points
- _`%w` notation_: for arrays of strings. Link to [styleguide](https://github.com/rubocop-hq/ruby-style-guide#percent-w)
- _chaining methods_ after a block (especially when people use a local variable for `join.to_i`
- _`each` vs `map`_: this explains the difference: [each vs map](https://learn.onemonth.com/ruby-tutorial-map-vs-each/)
- _variable vs constant_: this article explains just that: [var vs constant](https://ruby-doc.org/docs/ruby-doc-bundle/Tutorial/part_01/variables.html)
- _freeze_: this post has exactly the right level of information: [freeze](https://freelancing-gods.com/2017/07/27/an-introduction-to-frozen-string-literals.html); no need to dive deeper into (im)mutability.


### Changelog
First introduced 2019 Mar 10
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should have a specific format for the changelog. Maybe something like:

  • Version 1: exercise first introduced on March 10, 2019

Maybe HighScores can be a good guide as well?