plotly.kt
plotly.kt copied to clipboard
Make API accept named parameters instead of receiver lambdas when appropriate
Currently in many places the API looks like:
foo {
param1 = 2
param2 = "hello"
...
}
For example:
xaxis {
name = "x axis name"
mode = Mode.lines
}
The api would be easier to use if it was in this form:
xaxis = Axis(
title = "sin"
type= Type.`-`
)
There are a couple of upsides to this:
- By looking at the function signature of
xaxis{}
you don't know what kind of configuration is available. Sometimes the available variables are not even in the same file as the builder. When havingAxis()
accept the parameters directly, they are available immediately in the call site with their default values, and will be shown by IntelliJ when you type the constructor. - With named parameters IntelliJ will nicely autocomplete all of the available parameters, unlike when using lambdas.
- Having a function body implies you can do something other than assign variables, which is not true in many cases.
- Calling the constructor of
Foo
is the natural way to create an object of typeFoo
, instead of someFoo.build{}
function.
Places where passing parameters in receiver lambdas is still appropriate:
- When additional function calls are nested, like
plot
inPlotly.Page{}
(title
should still be a named parameter though), ortrace
inplot
.
@altavir If you agree I would like to submit a PR to start improving the API in this direction.
The changes you propose could be implemented without any change in API with extension function.
For example, you can add this code:
fun Axis(title: String, type: Type) = Axis.build{
this.title = title
this.type = type
}
This extension will allow to use xaxis = Axis(title = "sin", type= Type.
-)
inside layout builder. Personally, I think it is a step back in API design, but no harm done, so feel free to add the functionality you want. Just please do not add secondary constructors to Specific
classes, because it could make the code much harder to maintain. Maybe you should create a separate file for such extensions. I will probably move existing helpers like this one to extensions in future as well. Jost to separate API.
My point is that parameter-based builders should be the only API in certain cases. Could you explain the benefits of using receivers in Axis
for example?
@natanfudge There is no way around it. Basically, any creation of specification is the application of a function to an empty tree. So all builders you see are functions, objects. You can add a constructor-like front to the Specification::build
call, but underneath it still has functional nature and will use Axis.()->Unit
block somewhere inside.
Besides, the current approach is much better. It makes the code more flexible by allowing to represent parts of the logic as a function and move it into another place. Since everything is a function, you can easily add new behavior to existing API without breaking anything and without introducing a lot of unnecessary parameter overrides. The current programming trend is to move from imperative to functional definition in all places, where it is possible and I agree with it.
Could you elaborate, why do you not like the functional definition beside the point, it does not work like in python?
Could you elaborate, why do you not like the functional definition beside the point, it does not work like in python?
- By looking at the function signature of
xaxis{}
you don't know what kind of configuration is available. Sometimes the available variables are not even in the same file as the builder. When havingAxis()
accept the parameters directly, they are available immediately in the call site with their default values, and will be shown by IntelliJ when you type the constructor.- With named parameters IntelliJ will nicely autocomplete all of the available parameters, unlike when using lambdas.
- Having a function body implies you can do something other than assign variables, which is not true in many cases.
- Calling the constructor of
Foo
is the natural way to create an object of typeFoo
, instead of someFoo.build{}
function.
Well, let's discuss it.
-
The convention is (or will be) that the function calls a builder of appropriate specification. Some builder have a mandatory parameter, in those cases parameters could be passed outside lambda. This way is widely used in Kotlin libraries, so there should be no problems with that. The autocomplete works quite well in this case.
-
As I already said, auto-complete works perfectly well with receiver. You won't have positional arguments and good riddance, they could only confuse the code in this case.
-
You could. For example you can write some delayed or regular change right inside DSL, which is convenient. Consider this:
xaxis{
GlobalScope.launch{
while(true){
title = "Current time is ${Instant.now()}"
delay(500)
}
}
}
- But in this case constructor is not natural. When you write
xaxis = axis
in this case you do not assign value to a field, because there is no backing field forxaxis
. In fact, what you do is application of transformation (function) to the state tree.
You don't get this
You don't get this
In this case:
Even if you enter the source code (which you shouldn't need to do), you still wouldn't get to see what you can pass in. You need to enter the definition of Axis
, and by this point you are digging through the source code to see what you can pass.
Yes you can have external documentation that tells you what you can pass but you can and should have code that self documents.
Here type is explicit in the simplest way possible:
Here, yes you can technically infer that the API wants a string from the naming of string()
, but it's not explicit or simple
(what does
by string()
mean? If the only thing it meant was that you can pass a string then it wouldn't use property delegation - is what the user thinks)
Same thing with the default value,
Here I know the default value is null.
Here I assume/guess the default value is null
Does the dataforge library do something in this context other than serialize json? If not, then the declaration of classes can be simplified by using kotlinx.serialization
.
Regrading dynamic change, something like this can work well enough:
fun createPlot() = Plotly.plot {
trace(points = points, mode = TraceMode.`lines+markers`)
layout(
xaxis = Axis(title = "Current time is ${Instant.now()}"),
yaxis = Axis(title = "My y values")
)
}
fun main() {
GlobalScope.launch {
while (true) {
val plot = createPlot()
/* Use plot*/
delay(500)
}
}
}
And is even more natural when change comes externally, instead of being forced by the plot itself (like in the example you gave)
fun createPlot(data : Int) = Plotly.plot {
trace(points = points, mode = TraceMode.`lines+markers`)
layout(
xaxis = Axis(title = "I'm displaying data: $data"),
yaxis = Axis(title = "My y values")
)
}
fun onButtonPress() {
val plot = createPlot(getDataFromSomewhere())
/* Use plot */
}
To sum up:
- I'm not saying to not make it declarative, I'm saying to use parameters when parameters are appropriate.
- The losses in user experience in not using proper parameters are so huge that any gains in programming style (which I argue are non-existent, since we can still write declaratively, see Jetpack Compose and SwiftUI) are not worth it.
As it stands, when you use new function from the API, you either have to dig through code, or find documentation, which is by far the biggest issue.
Just write xaxis{
or Axis.build{
and you will get exactly the same level of autocomplete. You can also write this.
inside and get even more specific hints. It is probably a good idea to mark those functions with @DslMarker
to avoid autocapture of this
from external context.
The inner delegates like string()
could not be avoided because it is how the library works, it does not create explicit object with fields, it delegates everything to dynamic tree. We could add explicit types to those delegates for readability like var title: String? by string()
.
There is also one important thing that could not be done in a constructor-like pattern, namely nested objects. Currently, we've implemented a tiny part of plotly functionality, but there are a lot of places with nested objects creation. Implementation of such things with constructors are really ugly.
Please note, that I do not say that there should not be constructor-like builders. I've explained how to add them above, but it is not possible (and does not make sense) to make this way a primary way. Let me propose a compromise. You can add functions like this to all model classes (not inside the class, but as extension in the same file):
fun Axis(title: String, type: Type = Type.`-`, block: Axis.() ->Unit = {} ) = Axis.build{
this.title = title
this.type = type
}.apply(block)
This way, you keep both, constructor-like behavior and ability to add new fields and inner classes without breaking the API by optional lambda in the end. From user perspective, those functions will behave exactly the same way, you want. No additional imports are requires since they will be placed in the same file as actual classes.