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

Approvable solutions for Acronym #11

Open
emcoding opened this issue Jun 26, 2019 · 4 comments
Open

Approvable solutions for Acronym #11

emcoding opened this issue Jun 26, 2019 · 4 comments

Comments

@emcoding
Copy link

@iHiD

Per your request, here are the approvables for Acronym.
(Note that Acronym is not meeting all the requirements for a Level 1 exercise, (it's controversial, and it requires too many iterations to get it right), so when I find a better candidate it will move up in the track.)

  • Things that will be addressed later in the track, not on this level:

    • class or module : both approvable without comment
    • self. or class << self : both approvable without comment
  • Good solutions with split are auto-approvable; it would be nice to add a comment to investigate scan with regex /\b\w/.

  • Good solutions that have a too generic or typing name (input, string, str) for the param should preferrably have a comment about naming things, suggesting words or phrase.

  • One complete solution, approvable:

class Acronym
  def self.abbreviate(words)
    words.tr('-', ' ').split.map(&:chr).join.upcase
  end
end

Other approvables, method body only:

  • words.tr('-', ' ').split.map { |word| word.chr }.join.upcase

    • Symbol#to_proc is not relevant to this level, so the long form is auto-approvable.
    • Ignore spacing in block ({|word|...}) unless it's inconsistent : { |word| do_things}
    • Little bit controversial, but I would ignore double vs single quotes, especially if everything else is good. I'd mention it only when it's one of 3 or more style issues.
  • words.scan(/\b[[:alpha:]]/).join.upcase

    • plus other regexes: /\b\w/, /\b[a-zA-Z]/ , /\b[a-z]/i
    • (there are multiple regex's possible, most are covering not tested cases)
  • words.split(/[ -]/).map(&:chr).join.upcase

    • variants: split(/\W/),

Especially for this exercise, it may be interesting to see if non-approvables are easier to spot:

  • it will have word[0] within the block or word.slice(0)
  • and/or upcase is not at the end of the chain (note: there's one approvable that has upcase earlier).
  • and/or it will use gsub
  • and/or using each
  • and/or more than one line
  • plus a final check for join(
@emcoding emcoding changed the title Approvables in Ruby Approvables in Ruby: Acronym Jun 26, 2019
@iHiD iHiD transferred this issue from exercism/automated-analysis Jul 3, 2019
@iHiD
Copy link
Member

iHiD commented Jul 3, 2019

This is great! Moving to the ruby-analyzer repo.

@iHiD iHiD changed the title Approvables in Ruby: Acronym Approvable solutions for Acronym Jul 3, 2019
@emcoding
Copy link
Author

emcoding commented Aug 5, 2019

@iHiD Acronym is going to be moved shortly. And Resistor Color Duo is currently the first core after TwoFer, but the series is still experimental. Meaning: we need to evaluate the series as a whole and each exercise in the series. Plus since it's a new exercise, it is still 'volatile' and I expect it to change a few times in the next weeks and months.

@iHiD
Copy link
Member

iHiD commented Aug 5, 2019

@F3PiX We need to think about this. As a lot of effort is going into writing analyzers for these exercises, we need to co-ordinate amongst the tracks that have analyzers to ensure we're not wasting a lot of energy. Something to chat about on Wed maybe.

@emcoding
Copy link
Author

emcoding commented Aug 8, 2019

@iHiD Did the work on Acronym already start? Because then I'll find a different solution.

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

No branches or pull requests

2 participants