cli, sync: prefer Foo.init object construction style (#499)

Refactor our object construction style everywhere to be consistent with
that in the newer sync code.

There isn't a consensus on Nim object construction style, but the style
in this commit does have some objective advantages. For example, during
refactoring: to rename a type from `Foo` to `Bar`, we no longer need to
rename the constructor from `initFoo` to `initBar`.

See also: https://github.com/status-im/nim-style-guide/blob/83f58f89bc8e/src/language.objconstr.md

src/cli.nim

@@ -328,19 +328,19 @@ func formatOpt(kind: CmdLineKind, key: string, val = ""): string =
     else:
       &"'{prefix}{key}'"
 
-func initAction*(actionKind: ActionKind, probSpecsDir = "",
-                 scope: set[SyncKind] = {}): Action =
+func init*(T: typedesc[Action], actionKind: ActionKind, probSpecsDir = "",
+           scope: set[SyncKind] = {}): T =
   case actionKind
   of actNil, actFmt, actGenerate, actInfo, actLint:
-    Action(kind: actionKind)
+    T(kind: actionKind)
   of actSync:
-    Action(kind: actionKind, probSpecsDir: probSpecsDir, scope: scope)
+    T(kind: actionKind, probSpecsDir: probSpecsDir, scope: scope)
   of actUuid:
-    Action(kind: actionKind, num: 1)
+    T(kind: actionKind, num: 1)
 
-func initConf*(action = initAction(actNil), trackDir = getCurrentDir(),
-               verbosity = verNormal): Conf =
-  result = Conf(
+func init*(T: typedesc[Conf], action = Action.init(actNil),
+           trackDir = getCurrentDir(), verbosity = verNormal): T =
+  T(
     action: action,
     trackDir: trackDir,
     verbosity: verbosity,
@@ -417,8 +417,8 @@ proc parseVal[T: enum](kind: CmdLineKind, key: string, val: string): T =
 proc handleArgument(conf: var Conf; kind: CmdLineKind; key: string) =
   if conf.action.kind == actNil:
     let actionKind = parseActionKind(key)
-    let action = initAction(actionKind)
-    conf = initConf(action, conf.trackDir, conf.verbosity)
+    let action = Action.init(actionKind)
+    conf = Conf.init(action, conf.trackDir, conf.verbosity)
   else:
     showError(&"invalid argument for command '{conf.action.kind}': '{key}'")
 
@@ -506,7 +506,7 @@ proc handleOption(conf: var Conf; kind: CmdLineKind; key, val: string) =
                 &"{formatOpt(kind, key)}")
 
 proc processCmdLine*: Conf =
-  result = initConf()
+  result = Conf.init()
 
   for kind, key, val in getopt(shortNoVal = shortNoVal, longNoVal = longNoVal):
     case kind

src/sync/exercises.nim

@@ -11,6 +11,8 @@ type
     json*: ProbSpecsTestCase
     reimplements*: Option[ExerciseTestCase]
 
+  ExerciseTestCases = seq[ExerciseTestCase]
+
   ExerciseTests* {.requiresInit.} = object
     included*: HashSet[string]
     excluded*: HashSet[string]
@@ -22,18 +24,19 @@ type
   Exercise* {.requiresInit.} = object
     slug*: PracticeExerciseSlug
     tests*: ExerciseTests
-    testCases*: seq[ExerciseTestCase]
+    testCases*: ExerciseTestCases
 
-func initExerciseTests: ExerciseTests =
-  ExerciseTests(
+func init(T: typedesc[ExerciseTests]): T =
+  T(
     included: initHashSet[string](),
     excluded: initHashSet[string](),
     missing: initHashSet[string](),
   )
 
-proc initExerciseTests(practiceExerciseTests: PracticeExerciseTests,
-                       probSpecsTestCases: seq[ProbSpecsTestCase]): ExerciseTests =
-  result = initExerciseTests()
+proc init(T: typedesc[ExerciseTests],
+          practiceExerciseTests: PracticeExerciseTests,
+          probSpecsTestCases: ProbSpecsTestCases): T =
+  result = ExerciseTests.init()
   for testCase in probSpecsTestCases:
     let uuid = uuid(testCase)
     if uuid in practiceExerciseTests.included:
@@ -43,28 +46,28 @@ proc initExerciseTests(practiceExerciseTests: PracticeExerciseTests,
     else:
       result.missing.incl uuid
 
-proc newExerciseTestCase(testCase: ProbSpecsTestCase): ExerciseTestCase =
-  ExerciseTestCase(
+proc new(T: typedesc[ExerciseTestCase], testCase: ProbSpecsTestCase): T =
+  T(
     uuid: uuid(testCase),
     description: description(testCase),
     json: testCase,
   )
 
-proc getReimplementations(testCases: seq[ProbSpecsTestCase]): Table[string, string] =
+proc getReimplementations(testCases: ProbSpecsTestCases): Table[string, string] =
   for testCase in testCases:
     if testCase.isReimplementation():
       result[testCase.uuid()] = testCase.reimplements()
 
-func uuidToTestCase(testCases: seq[ExerciseTestCase]): Table[string, ExerciseTestCase] =
+func uuidToTestCase(testCases: ExerciseTestCases): Table[string, ExerciseTestCase] =
   for testCase in testCases:
     result[testCase.uuid] = testCase
 
-proc initExerciseTestCases(testCases: seq[ProbSpecsTestCase]): seq[ExerciseTestCase] =
+proc init(T: typedesc[ExerciseTestCases], testCases: ProbSpecsTestCases): T =
   result = newSeq[ExerciseTestCase](testCases.len)
   var hasReimplementation = false
 
   for i, testCase in testCases:
-    result[i] = newExerciseTestCase(testCase)
+    result[i] = ExerciseTestCase.new(testCase)
     if testCase.isReimplementation():
       hasReimplementation = true
 
@@ -85,13 +88,13 @@ iterator findExercises*(conf: Conf, probSpecsDir: ProbSpecsDir): Exercise {.inli
       let testCases = getCanonicalTests(probSpecsDir, practiceExercise.slug.string)
       yield Exercise(
         slug: practiceExercise.slug,
-        tests: initExerciseTests(practiceExercise.tests, testCases),
-        testCases: initExerciseTestCases(testCases),
+        tests: ExerciseTests.init(practiceExercise.tests, testCases),
+        testCases: ExerciseTestCases.init(testCases),
       )
     else:
       yield Exercise(
         slug: practiceExercise.slug,
-        tests: initExerciseTests(),
+        tests: ExerciseTests.init(),
         testCases: @[],
       )
 

src/sync/probspecs.nim

@@ -8,6 +8,8 @@ type
 
   ProbSpecsTestCase* = distinct JsonNode
 
+  ProbSpecsTestCases* = seq[ProbSpecsTestCase]
+
 proc `$`(dir: ProbSpecsDir): string {.borrow.}
 proc dirExists(dir: ProbSpecsDir): bool {.borrow.}
 proc removeDir*(dir: ProbSpecsDir, checkDir = false) {.borrow.}
@@ -36,7 +38,7 @@ func isReimplementation*(testCase: ProbSpecsTestCase): bool =
 proc reimplements*(testCase: ProbSpecsTestCase): string =
   testCase["reimplements"].getStr()
 
-proc initProbSpecsTestCases(node: JsonNode, prefix = ""): seq[ProbSpecsTestCase] =
+proc init(T: typedesc[ProbSpecsTestCases], node: JsonNode, prefix = ""): T =
   ## Returns a seq of every individual test case in `node` (flattening). We
   ## alter each `description` value to indicate any nesting, which is OK because
   ## we only use the `description` for writing `tests.toml`.
@@ -52,7 +54,7 @@ proc initProbSpecsTestCases(node: JsonNode, prefix = ""): seq[ProbSpecsTestCase]
       else:
         prefix
     for childNode in node["cases"].getElems():
-      result.add initProbSpecsTestCases(childNode, prefix)
+      result.add ProbSpecsTestCases.init(childNode, prefix)
 
 proc grainsWorkaround(grainsPath: string): JsonNode =
   ## Parses the canonical data file for `grains`, replacing the too-large
@@ -63,17 +65,19 @@ proc grainsWorkaround(grainsPath: string): JsonNode =
     ("184467440737095516", "184467440737095516.0"))
   result = parseJson(sanitised)
 
-proc parseProbSpecsTestCases(probSpecsExerciseDir: ProbSpecsExerciseDir): seq[ProbSpecsTestCase] =
+proc parseProbSpecsTestCases(probSpecsExerciseDir: ProbSpecsExerciseDir): ProbSpecsTestCases =
   ## Parses the `canonical-data.json` file for the given exercise, and returns
   ## a seq of, essentially, the JsonNode for each test.
   let canonicalJsonPath = canonicalDataFile(probSpecsExerciseDir)
-  if slug(probSpecsExerciseDir) == "grains":
-    canonicalJsonPath.grainsWorkaround().initProbSpecsTestCases()
-  else:
-    canonicalJsonPath.parseFile().initProbSpecsTestCases()
+  let j =
+    if slug(probSpecsExerciseDir) == "grains":
+      canonicalJsonPath.grainsWorkaround()
+    else:
+      canonicalJsonPath.parseFile()
+  result = ProbSpecsTestCases.init(j)
 
 proc getCanonicalTests*(probSpecsDir: ProbSpecsDir,
-                        slug: string): seq[ProbSpecsTestCase] =
+                        slug: string): ProbSpecsTestCases =
   ## Returns a seq of the canonical tests for the exercise `slug` in
   ## `probSpecsDir`.
   let probSpecsExerciseDir = joinPath(probSpecsDir.string, "exercises",
@@ -145,11 +149,11 @@ proc validate(probSpecsDir: ProbSpecsDir, conf: Conf) =
         showError("the given problem-specifications directory is not " &
                   &"up-to-date: '{probSpecsDir}'")
 
-proc initProbSpecsDir*(conf: Conf): ProbSpecsDir =
+proc init*(T: typedesc[ProbSpecsDir], conf: Conf): T =
   if conf.action.probSpecsDir.len > 0:
-    result = ProbSpecsDir(conf.action.probSpecsDir)
+    result = T(conf.action.probSpecsDir)
     validate(result, conf)
   else:
-    result = ProbSpecsDir(getCurrentDir() / ".problem-specifications")
+    result = T(getCurrentDir() / ".problem-specifications")
     removeDir(result)
     cloneExercismRepo("problem-specifications", result.string, shallow = true)

src/sync/sync.nim

@@ -91,7 +91,7 @@ proc syncImpl(conf: Conf): set[SyncKind] =
     if conf.action.scope == {skFilepaths}:
       ProbSpecsDir("this_will_not_be_used")
     else:
-      initProbSpecsDir(conf)
+      ProbSpecsDir.init(conf)
 
   try:
     let psExercisesDir = probSpecsDir / "exercises"

src/sync/tracks.nim

@@ -54,16 +54,16 @@ proc getPracticeExerciseSlugs(trackDir: TrackDir): seq[PracticeExerciseSlug] =
 
   sort result
 
-func initPracticeExerciseTests: PracticeExerciseTests =
-  PracticeExerciseTests(
+func init(T: typedesc[PracticeExerciseTests]): T =
+  T(
     included: initHashSet[string](),
     excluded: initHashSet[string](),
   )
 
-proc initPracticeExerciseTests(testsPath: string): PracticeExerciseTests =
+proc init(T: typedesc[PracticeExerciseTests], testsPath: string): T =
   ## Parses the `tests.toml` file at `testsPath` and returns HashSets of the
   ## included and excluded test case UUIDs.
-  result = initPracticeExerciseTests()
+  result = PracticeExerciseTests.init()
   if fileExists(testsPath):
     let tests = parsetoml.parseFile(testsPath)
 
@@ -96,10 +96,10 @@ iterator findPracticeExercises*(conf: Conf): PracticeExercise {.inline.} =
         let testsPath = testsPath(trackDir, slug)
         yield PracticeExercise(
           slug: slug,
-          tests: initPracticeExerciseTests(testsPath),
+          tests: PracticeExerciseTests.init(testsPath),
         )
       else:
         yield PracticeExercise(
           slug: slug,
-          tests: initPracticeExerciseTests(),
+          tests: PracticeExerciseTests.init(),
         )

tests/test_probspecs.nim

@@ -30,9 +30,9 @@ proc main =
         of psFresh: ""
         of psExisting: existingDir
 
-      let action = initAction(actSync, probSpecsPath)
-      let conf = initConf(action)
-      let probSpecsDir = initProbSpecsDir(conf)
+      let action = Action.init(actSync, probSpecsPath)
+      let conf = Conf.init(action)
+      let probSpecsDir = ProbSpecsDir.init(conf)
       let probSpecsExercises = getProbSpecsExercises(probSpecsDir)
 
       test "can return the exercises":

tests/test_tracks.nim

@@ -12,7 +12,7 @@ proc main =
                       "736245965db724cafc5ec8e9dcae83c850b7c5a8") # 2021-10-22
 
     let conf = Conf(
-      action: initAction(actSync, scope = {skTests}),
+      action: Action.init(actSync, scope = {skTests}),
       trackDir: trackDir,
     )
     let practiceExercises = toSeq findPracticeExercises(conf)