Use new builder pattern

[dryajov]

No description provided.

[sinkingsugar]

@dryajov Ah! thanks I had this on my todo list after I merged but completely forgot 😞

[zah]

Hrm, this pattern tends to be popular in languages that lack keyword parameters with default values.

[dryajov]

It can definitely be replaced with different overloads and default parameters, but that quickly gets messy and it's hardly extensible and maintainable. Looking at methods with dozens of parameters is not a pleasant exercise, let alone making changes such as deprecating arguments/options, etc, etc...

Ultimately, I think this is more a matter of taste and style, but the more declarative patterns like this, tend to have some specific readability and maintainability benefits, not unlike DSLs in some cases ;-)

[zah]

Let's try to list some objective merits of the different approaches:

1. The builder style cannot express required parameters at compile-time (this looks like a major short-coming).

2. A proc with defaults cannot express deprecated parameters at compile-time (a thin macro wrapper can though).

3. I would argue that both methods are equally extensible, because every new parameter must appear as a field in the builder. It would have been more extensible if for example the open/closed principle was in play, allowing third-party libraries to add new parameters to the builder.

The two styles will produce different output in doc generators and will behave differently with code assistance (these may be a matter of preference).

[dryajov]

The builder style cannot express required parameters at compile-time (this looks like a major short-coming).

Can't we make this compile time procs/templates and have all the proper checks run at compile time? I'm leaving this as a question, since this was the original idea but I'm really not sure how to make it happen in Nim.

A proc with defaults cannot express deprecated parameters at compile-time (a thin macro wrapper can though).

I'd love to see this macro and it's resulting usage.

I would argue that both methods are equally extensible, because every new parameter must appear as a field in the builder.

This is not something I'm arguing with, I'm making a case for the declarative syntax being more palatable and easier to grasp and more adhering to the principle of least surprise, which I'm afraid a lot of macro approaches fail at.

It would have been more extensible if for example the open/closed principle was in play, allowing third-party libraries to add new parameters to the builder.

I'm not sure how this affects the open/closed principle, the builder is still extensible with new fields and toggler procs through either inheritance or delegation?

[zah]

Can't we make this compile time procs/templates and have all the proper checks run at compile time? I'm leaving this as a question, since this was the original idea but I'm really not sure how to make it happen in Nim.

What I meant is that nothing prevents the user from skipping some of your parameters:

var switch = SwitchBuilder.new()
      .withAddress(address)
      .withRng(rng)
      .build()

It would be difficult to detect this at compile time (it would require a quite complicated/ugly technique called session types). Admittedly, It's trivial to detect at run-time with an assert in build(), but to answer you specific question, I don't see how you can run this assert at compile-time because the entire expression must accept at least some run-time parameters.

I'd love to see this macro and it's resulting usage.

Such a macro wouldn't be very complicated actually. It will be used as a decorator pragrma:

proc foo(bar {.deprecated.} = 0, baz = "baz") {.withDeprecatedParams.}
# This renames `foo` and creates a thin substitute wrapper that checks
# whether the `bar` param was used.

It's quite possible that Nim will eventually add native support for deprecated parameters in the future making the pragma a no-op.

This is not something I'm arguing with, I'm making a case for the declarative syntax being more palatable and easier to grasp and more adhering to the principle of least surprise, which I'm afraid a lot of macro approaches fail at.

I would argue that calling a constructor with named parameters is equally declarative. One way to measure the "declarative-ness" of an API is to count the degrees of freedom (less degrees of freedom => more declarative API). The regular constructor has less degrees of freedom precisely because it can rule out some invalid ways of constructing the object (i.e. missing a required parameter, providing the same parameter multiple times, etc).

I'm not sure how this affects the open/closed principle, the builder is still extensible with new fields and toggler procs through either inheritance or delegation?

So is a regular proc when wrapped in another proc that adds additional parameters.

---

Ultimately, one has to be careful when copying styles from other ecosystems. The builder pattern is quite popular in JavaScript and this makes it familiar to you, but the reason it's popular there is that JavaScript doesn't have named parameters with default values as a language feature and the builder pattern is a good way to simulate them.

[dryajov]

Ultimately, one has to be careful when copying styles from other ecosystems. The builder pattern is quite popular in JavaScript and this makes it familiar to you, but the reason it's popular there is that JavaScript doesn't have named parameters with default values as a language feature and the builder pattern is a good way to simulate them.

I think that you just made my point better than what I could have - javascript turns out being quite a popular language, FWIW it's #7 on the tiobe index (https://www.tiobe.com/tiobe-index/). This is an audience that Nim partly caters to by offering browser/javascript interop and it only makes sense to make that transition smoother; but in reality this is a widely used pattern in Java, C#, C++, Go, Rust and just about any high level language.

But let's get back to the specific merits of builders vs constructor functions.

What I meant is that nothing prevents the user from skipping some of your parameters:

Constructor functions aren't any better at expressing different complex object creation paths, in fact they quickly become more complex and harder to maintain because to have the compile time guarantees that you bring up, there has to be many versions of the constructor with many different combinations of parameters. What's worst, since the only way to achieve this is with overloading, more often than not, there is a default constructor that makes all/most parameters as optional, which means that all the compile times guarantees are also lost.

Let's take this simple constructor proc:

proc newMyObject(T: type MyObject, param1: string, param2: string, param3: string): MyObject =
  ...

You can use it as:

let obj = MyObject.newMyObject("I'm", "an", "object")

Now, lets say that we want to make the 3rd parameter optional, in nim we can have optional parameters so that gives us two potential ways of doing that - overloads or default/optional params

Default param:

proc newMyObject(T: type MyObject, param1: string, param2: string, param3: string = ""): MyObject =
  ...

In this case, you loose the compile time guarantees (one reason that languages like Java don't allow default/optional params).

Overloads:

proc newMyObject(T: type MyObject, param1: string, param2: string): MyObject =
  MyObject.newMyObject(param1, param2, "")

In this case, you now have two versions of the constructor to maintain. Now, extrapolate that to dozens of parameter combinations and overloads quickly become impractical.

Such a macro wouldn't be very complicated actually. It will be used as a decorator pragrma:

This only partially addresses parameter deprecation, but it does nothing for the issues described above.

I would argue that calling a constructor with named parameters is equally declarative. One way to measure the "declarative-ness" of an API is to count the degrees of freedom (less degrees of freedom => more declarative API).

Sure, not all is lost with constructor functions and having named parameters is definitely very useful, but they aren't a direct replacement for the builder pattern as they still don't address the issues I described above.

The regular constructor has less degrees of freedom precisely because it can rule out some invalid ways of constructing the object (i.e. missing a required parameter, providing the same parameter multiple times, etc).

In some cases, but in others it gives you a false sense of safety, specially when using named params, which tend to have default values.

The builder pattern stands on its own as a way to express complex object construction. It isn't there because Java or javascript lacked named/default params, C# for example allows for named parameters (https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/named-and-optional-arguments) and yet it's also widely used in that ecosystem.

[zah]

The style I'm advocating for such complex constructors is to use only named parameters:

var x = MyObject.new(param2 = "foo",
                     param1 = "bar",
                     logger = FileLogger.new("my.log"))
                     
# which is equivalent to:

var x = MyObjectBuilder
   .new()
   .withParam2("foo")
   .withParam1("bar")
   .withFileLogger("my.log")
   .build()

Please note that the parameters can appear in any order and the compilation rules will give us the following properties:

1. The names and types of the parameters must match the expected API (which has a single entry in the documentation).
2. All required parameters must be provided.
3. There might be optional parameters with default values.
4. You need only a single constructor definition that is equivalent to the build function.

Arguably, the builder pattern is doing slightly worse on point 1 due to the weaker documentation, it completely fails to deliver point 2 and it requires more code to express point 3 and 4.

You claim that one would need many overloads of the constructor to match the expressivity of the builder pattern and I can assume this is coming from the potential of having "branches" in the constructor - the user is either constructing a Mplex-based switch, a Yamux-based switch (which has a similar role, but requires different parameters) or a QUIC switch (which has a different role to some extent). My example is using a hypothetical polymorphic logger as an illustration for how this can be done, but I would acknowledge that more complex branching logic cannot be expressed correctly. I would claim though that you will have an equivalent problem with the builder pattern - all the possible branches will have to be expressed as field in the builder object and the build function will be responsible for ruling out the invalid configurations only at run-time.

Now, you can argue that my preferred style is not enforced by the compiler (as one can still use positional arguments), but it would be natural for users of such complex constructors to prefer it. Furthermore, enforcement is actually possible if we rely again on the thin wrapper macro that can ensure that all call sites are using only keyword arguments.

From all the languages you've listed, only C# has proper keyword arguments and they were added in C# 4 (which is admittedly quite old). APIs in the .NET ecosystem suffer from another problem though - they must be accessible from any CLR language which makes relying on C# features less practical.

[dryajov]

Btw, any blockers to get this merged besides the points on style/pattern used by libp2p?