-
-
Notifications
You must be signed in to change notification settings - Fork 531
[WIP] generator: refactor implementation, part 1 #641
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
Changes from all commits
2778649
ac3b558
39e5dae
3d33f3c
96c47ea
8f870a6
e2b41c2
892acf2
d0e4205
9dfc1f8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| module Generator | ||
| class Exercise | ||
| using Generator::Underscore | ||
|
|
||
| attr_reader :slug | ||
|
|
||
| def initialize(slug:) | ||
| @slug = slug | ||
| end | ||
|
|
||
| def name | ||
| @name ||= slug.underscore | ||
| end | ||
|
|
||
| def case_class | ||
| slug.camel_case + 'Case' | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,11 +3,9 @@ module Files | |
| module GeneratorCases | ||
| class << self | ||
| def available(track_path) | ||
| cases_filepaths(track_path).map { |filepath| slugify(filepath) }.sort | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I strongly disagree with this change. It is true that available does not need to know exactly how a test case is generated, and you could achieve this by abstracting out the availability criteria, but since that is all this class does that seems excessive.
This is a case of YAGNI and we can worry about that when it happens. See also: #646 which aims to give new generator implementers helpful advice. Request: Omit this commit. |
||
| end | ||
|
|
||
| def class_name(exercise_name_or_slug) | ||
| filename(exercise_name_or_slug).split('_').map(&:capitalize).join | ||
| filepaths(track_path).map do |filepath| | ||
| %r{#{track_path}/exercises/([-a-z]+)/}.match(filepath)[1] | ||
| end.sort | ||
| end | ||
|
|
||
| def source_filepath(track_path, slug) | ||
|
|
@@ -18,13 +16,8 @@ def source_filepath(track_path, slug) | |
|
|
||
| private | ||
|
|
||
| def cases_filepaths(track_path) | ||
| generator_glob = File.join(meta_generator_path(track_path, '*'), '*_case.rb') | ||
| Dir.glob(generator_glob, File::FNM_DOTMATCH) | ||
| end | ||
|
|
||
| def slugify(filepath) | ||
| File.basename(filepath, '_case.rb').tr('_', '-') | ||
| def filepaths(track_path) | ||
| Dir.glob(meta_generator_path(track_path, '*')) | ||
| end | ||
|
|
||
| def filename(exercise_name_or_slug) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,55 +1,63 @@ | ||
| module Generator | ||
| # Contains methods accessible to the ERB template | ||
| class TemplateValues | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The original intention of It is the responsibility of the If that is the goal, then there is probably a nicer way of implementing this but I'm not sure this commit is heading in that direction. |
||
| attr_reader :abbreviated_commit_hash, :version, :exercise_name, :test_cases, :canonical_data_version | ||
| using Underscore | ||
|
|
||
| def initialize(abbreviated_commit_hash:, version:, exercise_name:, test_cases:, canonical_data_version: nil) | ||
| @abbreviated_commit_hash = abbreviated_commit_hash | ||
| attr_reader :exercise, :version, :canonical_data, :test_cases | ||
|
|
||
| def initialize(exercise:, version:, canonical_data:, test_cases:) | ||
| @exercise = exercise | ||
| @version = version | ||
| @exercise_name = exercise_name | ||
| @canonical_data = canonical_data | ||
| @test_cases = test_cases | ||
| @canonical_data_version = canonical_data_version | ||
| end | ||
|
|
||
| def get_binding | ||
| binding | ||
| end | ||
|
|
||
| def abbreviated_commit_hash | ||
| canonical_data.abbreviated_commit_hash | ||
| end | ||
|
|
||
| def canonical_data_version | ||
| canonical_data.version | ||
| end | ||
|
|
||
| def exercise_name | ||
| exercise.name | ||
| end | ||
|
|
||
| def exercise_name_camel | ||
| exercise_name.split('_').map(&:capitalize).join | ||
| exercise.name.camel_case | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| end | ||
| end | ||
|
|
||
| module TemplateValuesFactory | ||
| def template_values | ||
| TemplateValues.new( | ||
| abbreviated_commit_hash: canonical_data.abbreviated_commit_hash, | ||
| canonical_data_version: canonical_data.version, | ||
| exercise: exercise, | ||
| version: version, | ||
| exercise_name: slug_underscore, | ||
| canonical_data: canonical_data, | ||
| test_cases: extract | ||
| ) | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def slug_underscore | ||
| slug ? slug.tr('-_', '_') : '' | ||
| end | ||
|
|
||
| def extract | ||
| load cases_load_name | ||
| extractor.cases(canonical_data.to_s) | ||
| end | ||
|
|
||
| def extractor | ||
| CaseValues::Extractor.new( | ||
| case_class: Object.const_get(Files::GeneratorCases.class_name(slug)) | ||
| ) | ||
| CaseValues::Extractor.new( | ||
| case_class: Object.const_get(exercise.case_class) | ||
| ) | ||
| end | ||
|
|
||
| def cases_load_name | ||
| Files::GeneratorCases.source_filepath(paths.track, slug) | ||
| Files::GeneratorCases.source_filepath(paths.track, exercise.slug) | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,10 @@ module Underscore | |
| def underscore | ||
| downcase.gsub(/[- ]/, '_').gsub(/[^\w?]/, '') | ||
| end | ||
|
|
||
| def camel_case | ||
| underscore.split('_').map(&:capitalize).join | ||
| end | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| require_relative '../test_helper' | ||
|
|
||
| module Generator | ||
| class ExerciseTest < Minitest::Test | ||
| def test_slug | ||
| exercise = Exercise.new(slug: 'alpha') | ||
| assert_equal 'alpha', exercise.slug | ||
| end | ||
|
|
||
| def test_name | ||
| exercise = Exercise.new(slug: 'alpha-beta') | ||
| assert_equal 'alpha_beta', exercise.name | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yay for tests. |
||
| end | ||
|
|
||
| def test_case_class | ||
| exercise = Exercise.new(slug: 'alpha-beta') | ||
| assert_equal 'AlphaBetaCase', exercise.case_class | ||
| end | ||
| end | ||
| end | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this 👍