Nim: [RFC] Keyword-Only Arguments; leads to more readable code and fwd-compatible changes in API's

Created on 13 Jul 2018  路  10Comments  路  Source: nim-lang/Nim

let's start with a quizz: which one of these is easier to understand ? (taken from @citycide's excellent glob library)

# from https://citycide.github.io/glob/
walkGlobKinds("*.*", true, true, false, false)
walkGlobKinds("*.*", root = "", relative = true, expandDirs = true, includeHidden = false; includeDirs = false)

Clearly, the 2nd one. The 1st one pretty much forces you to read the docs at every single such function call even if you're familiar with that API.

Yet, the 2nd one is not enforced by the compiler and relying on convention sucks.

  • in python, they solved this problem by introducing keyword only arguments PEP3102

The use case is to force callers to use keyword calling as opposed to positional calling for these arguments, with benefits:

  • can result in much more readable code, see above
  • gives API writer more freedom to evolve API (eg adding params in the middle or reshuffling), all without breaking any code
  • removal of params in will cause compilation error if caller calls these params as opposed to silently accept wrong code in some cases, see [1]
    NOTE: keyword-only arguments would make such API evolution and parameter deprecation much easier and less error prone (see https://github.com/citycide/glob/issues/11 [RFC] API simplification for example)

python's approach

here's python's syntax + example from https://stackoverflow.com/questions/2965271/forced-naming-of-parameters-in-python to illustrate:

>>> def foo(pos, *, forcenamed):
...   print(pos, forcenamed)
... 
>>> foo(pos=10, forcenamed=20)
10 20
>>> foo(10, forcenamed=20)
10 20
>>> foo(10, 20)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: foo() takes exactly 1 positional argument (2 given)

NOTE: relevant snippet from [PEP3102](https://www.python.org/dev/peps/pep-3102/ introduced keyword only arguments :

Keyword-only arguments are not required to have a default value. Since Python requires that all arguments be bound to a value, and since the only way to bind a value to a keyword-only argument is via keyword, such arguments are therefore 'required keyword' arguments. Such arguments must be supplied by the caller, and they must be supplied via keyword.

suggested syntax up for discussion:

we could use same syntax as python, nice and simple, but which particular syntax doesn't matter much

[1]

proc foo(a=100, b=20)=discard
foo(3)
foo(a=3) # with keyword-only arg

# later `a=100` is removed
proc foo(b=20)=discard
foo(3) # silently accepted with wrong semantics
foo(a=3)  # with keyword-only arg: better: compilation error
Language design RFC

Most helpful comment

Yet, the 2nd one is not enforced by the compiler and relying on convention sucks.

You know what sucks much more? A language that never stabilizes. -1 from me on yet another language feature.

walkGlobKinds("*.*", root = "", relative = true, expandDirs = true, includeHidden = false; includeDirs = false)

Should make use of Nim's set[enum] feature.

walkGlobKinds("*.*", root = "", {Glob.relative, Glob.expandDirs})

All 10 comments

The argument about making it easier to deprecate/add named parameters is really important IMO. It's currently unclear when it's "safe" to use named parameters when calling third-party code, since renaming a parameter is typically not treated as a breaking change. If the parameter can _only_ be used as a named parameter however, it's obvious that any change to the name is a breaking change.

This also opens up the possibility of having a deprecation path when a named parameter is renamed.

The case where a named parameter had to be added to the beginning of the parameter list has already happened in times.nim, and the solution was to introduce a new proc and deprecate the old...

Not a bad idea, but my favourite style is to reify the options:

```nim
type GlobOptions = object
relative, expandDirs, includeHidden, includeDirs: bool

walkGlobKinds(".", root = "", options=GlobOptions(relative: true))

problem with that is defaults become an all or nothing:

proc fun(pattern: string, options=GlobOptions(root:"/", relative: true, includeDirs: true)) = ...
fun("*.*") # uses all the defaults, ie GlobOptions(root:"/", relative: true, includeDirs: true)
# but if we want to override just 1 field (say, `relative: false`) and keep other defaults, it's not DRY:
fun("*.*", options=GlobOptions(root:"/", relative: false, includeDirs: true))

that reified style is actually what prompted me to write obj.updateObj(field1 = foo, field2 = "bar") (see RFC https://github.com/nim-lang/Nim/issues/8180 which has code for that), but still, usage is cumbersome:

proc initGlobOptions():auto=GlobOptions(root:"/", relative: true, includeDirs: true)
proc fun(pattern: string, options=initGlobOptions()) = ...
fun("*.*", options=initGlobOptions().updateObj(relative = false))
# instead of, without reified style:
fun("*.*", relative = false)

@andreaferretti Curious whether you have some good ideas on this...

That being said, this RFC is useful independently of that point

Yet, the 2nd one is not enforced by the compiler and relying on convention sucks.

You know what sucks much more? A language that never stabilizes. -1 from me on yet another language feature.

walkGlobKinds("*.*", root = "", relative = true, expandDirs = true, includeHidden = false; includeDirs = false)

Should make use of Nim's set[enum] feature.

walkGlobKinds("*.*", root = "", {Glob.relative, Glob.expandDirs})

unfortunately set won't help with non-bool arguments which is the more general case.
And in this particular example of glob library, root is such an argument, that would ideally only ever be specified by keyword, not by position.

Here's another typical example to that effect, from osproc.startProcess, where not all args fit in a set:

proc startProcess(command: string; workingDir: string = "";
                 args: openArray[string] = []; env: StringTableRef = nil;
                 options: set[ProcessOption] = {poStdErrToStdOut}): Process {..}

# this is obvious without reading docs
startProcess("bar", workingDir = "foo",  env = ["FOO" : "BAR"], options = {poStdErrToStdOut})
# this is not so obvious:
startProcess("bar", "foo", nil, ["FOO" : "BAR"], {poStdErrToStdOut})

@timotheecour a little more verbose, but one can do

var opt = defaultOptions()
opt.relative = true

walkGlobKinds("*.*", root = "", opt)

Also, if the proposal for default values for object fields gets accepted, the problem af all or nothing goes automatically away

Here's another typical example to that effect, from osproc.startProcess, where not all args fit in a set

Yes, I know, named arguments are awesome. Use them. You don't need enforcement in order to use something.

Yep the set[Enum] thing is what I had proposed last week for glob in https://github.com/citycide/glob/issues/11#issuecomment-403181152. I just started a WIP rework to implement that in https://github.com/citycide/glob/pull/19 if anyone is interested in checking it out.

So while I wouldn't be opposed to this feature I'm in agreement with @Araq, this is kind of unnecessary.

I agree with everything @Araq said. I didn't even know Python had this feature.

As such I think the consensus is to reject this and therefore close this issue (sorry).

Was this page helpful?
0 / 5 - 0 ratings