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/two-fer: Improve mentor notes #641

Open
wants to merge 9 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@SleeplessByte
Contributor

SleeplessByte commented Dec 2, 2018

After having an interaction with a mentor on a solution without any good explanation why the interaction was what it was, I've sifted through other solutions and mentors comments. These mentor-note changes reflect my findings and what I believe to be better in terms of the exercism learning experience.

Group style suggestions

Groups the suggestions about style together, so it's clear which suggestions are functional and which are not. Add links to the specific community style guide rules.

Remove explicit module notion and make it a first class citizen

At this point in the track it's fine to use ANYTHING, but the notion that "class" is somehow more correct than module is off-putting.

A class should only be used if its functionality requires instantation. Utility functions and the sorts should either be on a named constant which is an Object, or a module so it can be included/extended and shared in other modules and classes.

Add more reasonable solutions including string templates

The point of this exercise is to prepare a student for the track, not force one usage over another.

String interpolation is simpler, but template strings are much more maintainable when used with named arguments as it can be subtituted easily for internationalisation and/or extended.

This adds all the variations as reasonable so mentors no longer penalise students for using it.

SleeplessByte added some commits Dec 2, 2018

Group style suggestions
Groups the suggestions about style together, so it's clear which suggestions are functional and which are not. Add links to the specific community style guide rules.
Remove explicit module notion and make it a first class citizen
At this point in the track it's fine to use ANYTHING, but the notion that "class" is somehow more correct than module is off-putting.

A class should only be used if its functionality requires instantation. Utility functions and the sorts should either be on a named constant which is an Object, or a module so it can be included/extended and shared in other modules and classes.
Add more reasonable solutions including string templates.
The point of this exercise is to prepare a student for the track, not force one usage over another.

String interpolation is simpler, but template strings are much more maintainable when used with named arguments as it can be subtituted easily for internationalisation and/or extended.

This adds all the variations as reasonable so mentors no longer penalise students for using it.
Remove bookkeeper suggestion
I don't think this is still part of the actual implementation and thus does not need to be part of the mentor notes.
@kotp

A few suggestions for spelling/grammar.

SleeplessByte added some commits Dec 5, 2018

Change to use actual alternative
`format` is being used in the example, so `sprintf` is the alternative to that
@iHiD

This comment has been minimized.

Contributor

iHiD commented Dec 5, 2018

As the three approaches could all be mixed up, I'm not sure separating them into three makes sense.
I wonder if rather than having 3 solutions, it would be clearer to have one solution with comments. e.g.

# May use class instead of module
module TwoFer

  # May use module_function instead of self.
  def self.two_fer(name = 'you')

    # Or "format('One for %<name>s, one for me.', name: name)'"
    # Or "One for %s, one for me.' % name"
    "One for #{name}, one for me."
  end
end
@SleeplessByte

This comment has been minimized.

Contributor

SleeplessByte commented Dec 5, 2018

@iHiD I personally really don't like these types of comments in solutions, as they tend to clutter up, it's not what comments are meant for and only works okay for two-fer, but not for more complex exercises. In this case the comments are also missing the valid metaclass and sprintf-almost-but-not-quite-alias.

In general in the JS and TS tracks I've started writing "variations include" without examples. Other mentors requested me to write out those examples as they felt that to be more clear. If this were up to me I'd only put down one solutions and a list of "acceptable variations" and leave it up to mentor to fill in the blanks.

Ideally we'd have one of those "samples" boxes like this:
image

But then with multiple approaches as tabs or something like that :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment