Skip to content

generate: refactor slightly#618

Merged
ee7 merged 7 commits intoexercism:mainfrom
ee7:generate-refactor-slightly
Jun 19, 2022
Merged

generate: refactor slightly#618
ee7 merged 7 commits intoexercism:mainfrom
ee7:generate-refactor-slightly

Conversation

@ee7
Copy link
Copy Markdown
Member

@ee7 ee7 commented Jun 17, 2022 •
edited
Loading

Summary:

  • Rename index variable to i

  • Prefer in-place strip and slice

  • Remove an else branch

  • Explicitly initialize result - the previous pattern of result.add
    before setting result may become an error in a future version of Nim

  • DRY filename setting

Just splitting outsome tiny refactorings I did while working on the generate code.

I had some other refactorings here, but I'll leave them for later.

ee7 added 6 commits June 17, 2022 12:28
@ee7
generate: DRY filename setting
55d8cee 
Another way to remove exactly one file extension is:

    introductionTemplatePath.string.changeFileExt("")

but it's not immediately obvious that only one file extension is
removed.
@ee7
generate: rename index variable to i
074bb08 
Just to be consistent with elsewhere in the configlet codebase:

    $ git grep --heading --break 'for i[, ]' -- '*.nim'
    src/helpers.nim
    79:  for i, c in s:

    src/lint/track_config.nim
    403:    for i in 1..prereqPath.high:

    src/patched_stdlib/json.nim
    433:    for i in 0 ..< x.len:
    439:    for i in 0 ..< x.len:
    572:  for i in 0..<L:
    613:  for i in index:
    633:  for i in 0..(keys.len-2):
    666:    for i in items(p.elems):
    798:      for i in 0..len(node.elems)-1:
    842:  for i in items(node.elems):
    849:  for i in mitems(node.elems):
    1015:      for i in 0 ..< x.len:
    1176:    for i in 0 ..< jsonNode.len:
    1186:    for i in 0 ..< jsonNode.len:
    1304:      for i in 1 ..< typeNode.len:
    1309:      for i in 0 ..< typeNode.len-1:

    src/patched_stdlib/parsejson.nim
    255:          for i in 1 .. length:

    src/sync/exercises.nim
    69:  for i, testCase in testCases:

    src/sync/sync_common.nim
    34:  for i, item in e:
    151:    for i, item in val:

    src/sync/sync_docs.nim
    36:    for i in countdown(result.high, descHeader.len):
    38:    for i, c in instrHeader:

    src/sync/sync_filepaths.nim
    29:  for i, pattern in patterns:

    src/uuid/uuid.nim
    14:  for i in 1 .. numUuidsToGenerate:

    tests/test_fmt.nim
    207:        for i in 1..100:
    282:        for i in 1..100:

    tests/test_lint.nim
    74:    for i, c in ValidUuidV4:
    125:      for i in HexIndices:
    143:      for i in HyphenIndices:
    150:      for i in HyphenIndices:
    158:      for i in HexIndices:
    166:      for i in HyphenIndices:
    177:      for i in 0 .. uuid.high:

    tests/test_uuid.nim
    8:      for i in 1 .. 1000:
@ee7
generate: avoid substr
3fea191 
For the reader who isn't sure about what `substr` does when only two
arguments are passed, this is clearer that we're removing the beginning
of the string rather than the end.

However, we'll change this line later in this PR to slice in place.
@ee7
generate: prefer in-place strip
9691bfe 
Allocate less, DRY slightly, and allow further refactoring in the
next commits.
@ee7
generate: remove an else branch
d5a9c98
@ee7
generate: slice in place
f2e5975 
We now do an in-place `strip`, so let's do the same for slicing.
@ee7 ee7 requested a review from ErikSchierboom as a code owner June 17, 2022 10:33
@ee7 ee7 mentioned this pull request Jun 17, 2022
Merged
5 tasks
@ee7 ee7 mentioned this pull request Jun 17, 2022
Merged
4 tasks
@ee7
generate: explicitly initialize result
5daa244 
The previous pattern of `result.add` without setting `result` may become
an error in a future version of Nim.

The initial size isn't particularly important, but here are the concept
introduction.md file sizes for Elixir:

    $ wc -c $(git ls-files -- 'concepts/*/introduction.md')
     1142 concepts/access-behaviour/introduction.md
      890 concepts/agent/introduction.md
      472 concepts/alias/introduction.md
     1167 concepts/anonymous-functions/introduction.md
     1542 concepts/ast/introduction.md
      464 concepts/atoms/introduction.md
     3477 concepts/basics/introduction.md
     3078 concepts/behaviours/introduction.md
     1983 concepts/binaries/introduction.md
      393 concepts/bit-manipulation/introduction.md
     1713 concepts/bitstrings/introduction.md
      634 concepts/booleans/introduction.md
      386 concepts/case/introduction.md
      840 concepts/charlists/introduction.md
      477 concepts/cond/introduction.md
      947 concepts/dates-and-time/introduction.md
      804 concepts/default-arguments/introduction.md
     1041 concepts/docs/introduction.md
      493 concepts/dynamic-dispatch/introduction.md
     2473 concepts/enum/introduction.md
     1782 concepts/erlang-libraries/introduction.md
      654 concepts/errors/introduction.md
     1802 concepts/exceptions/introduction.md
     1161 concepts/file/introduction.md
     1149 concepts/floating-point-numbers/introduction.md
     7576 concepts/genserver/introduction.md
      587 concepts/guards/introduction.md
     1310 concepts/if/introduction.md
      554 concepts/import/introduction.md
      153 concepts/integers/introduction.md
     1112 concepts/io/introduction.md
      744 concepts/keyword-lists/introduction.md
     1514 concepts/links/introduction.md
     1000 concepts/list-comprehensions/introduction.md
     1030 concepts/lists/introduction.md
     1061 concepts/maps/introduction.md
      553 concepts/module-attributes-as-constants/introduction.md
      601 concepts/multiple-clause-functions/introduction.md
      377 concepts/nil/introduction.md
     1618 concepts/pattern-matching/introduction.md
      278 concepts/pids/introduction.md
      314 concepts/pipe-operator/introduction.md
     2509 concepts/processes/introduction.md
      654 concepts/protocols/introduction.md
      345 concepts/randomness/introduction.md
      375 concepts/ranges/introduction.md
      526 concepts/recursion/introduction.md
     3400 concepts/regular-expressions/introduction.md
      807 concepts/streams/introduction.md
      679 concepts/strings/introduction.md
     1738 concepts/structs/introduction.md
     1590 concepts/tail-call-recursion/introduction.md
     1385 concepts/tasks/introduction.md
      703 concepts/try-rescue-else-after/introduction.md
     1431 concepts/try-rescue/introduction.md
     1016 concepts/tuples/introduction.md
     2125 concepts/typespecs/introduction.md
     1483 concepts/use/introduction.md
      852 concepts/with/introduction.md
    72964 total
@ee7 ee7 merged commit d39169d into exercism:main Jun 19, 2022
@ee7 ee7 deleted the generate-refactor-slightly branch June 19, 2022 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ErikSchierboom ErikSchierboom ErikSchierboom approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ee7 @ErikSchierboom