-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make API accept named parameters instead of receiver lambdas when appropriate #12
Comments
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 |
My point is that parameter-based builders should be the only API in certain cases. Could you explain the benefits of using receivers in |
@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 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? |
|
Well, let's discuss it.
xaxis{
GlobalScope.launch{
while(true){
title = "Current time is ${Instant.now()}"
delay(500)
}
}
}
|
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 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 Same thing with the default value, 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 Regrading dynamic change, something like this can work well enough:
And is even more natural when change comes externally, instead of being forced by the plot itself (like in the example you gave)
To sum up:
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 The inner delegates like 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. |
Currently in many places the API looks like:
For example:
The api would be easier to use if it was in this form:
There are a couple of upsides to this:
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 having
Axis()
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.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:
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 text was updated successfully, but these errors were encountered: