Tornadofx: Sweep for non-optional `op` arguments

[thomasnield]

Documenting things right now and found something that might need to be addressed. Occasionally I am finding builders that require an op block which shouldn't always be necessary. For instance, calling piechart() won't compile unless I put a block { } after it. The op really should be nullable and optional.

val items = FXCollections.observableArrayList(
        PieChart.Data("Windows", 77.62),
        PieChart.Data("OS X", 9.52),
        PieChart.Data("Other", 3.06),
        PieChart.Data("Linux", 1.55),
        PieChart.Data("Chrome OS", 0.55)
)

piechart("Desktop/Laptop OS Market Share", items) {
   //this block is mandatory
}

I can't think of any other builders that have this problem. But it might be good if I do a sweep at some point.

[ruckustboom]

I did a quick search for all the functions I could fine that take an op argument that does not default to null. There are quite a few, and I'm sure several of them are by design (for example, all of them in CSS.kt do require a non-null block), but I'm not sure which are which, so here is a list of what I found.

I removed CSS.kt from the list as the op argument is required. If anyone can verify which of these are required and remove them, we can fix the rest.

Charts.kt

• Pane.piechart()
• CharType.multiseries()

Components.kt

• Component.preferences()

ItemControls.kt

• Pane.tableview()
• Pane.treeview()
• Pane.treetableview()

Layouts.kt

• GridPane.row()
• BorderPane.top()
• BorderPane.bottom()
• BorderPane.left()
• BorderPane.roght()
• BorderPane.center()
• Pane.scrollpane()
• SplitPane.items()
• Accordion.fold()

Libs.kt

• Clipboard.setContent()

Menu.kt

• MenuBar.menu()
• ContextMenu.menu()
• Menu.menu()
• Control.contextmenu()

Nodes.kt

• Toolbar.children()
• children()
• T.borderpaneConstraints()
• T.gridpaneConstraints()
• T.vboxConstraints()
• T.hboxConstraints
• T.anchorpaneConstraints()

[thomasnield]

Nice! Thanks I was going to do that this weekend, but I'm glad you beat me to it. I'll go through these and label which should be optional in my opinion. Then we can discuss from there before editing.

[edvin]

Great work guys! And smart work flow. I'll hold off until Thomas has made the list and then we'll figure out which should be made optional. Thanks for picking up the slack while I'm offline guys, you're amazing!

[thomasnield]

I'm going to follow up this weekend on this. My folks are in San Diego next week. I'm going to take advantageof it and get some loose ends done.

[edvin]

Thomas did you want to have a go at this? If you don't have the time I can look at it this weekend :)

[thomasnield]

Sorry been suddenly swamped. I'll see if I can get it done in the next two days. I'll definitely be free this weekend.

[edvin]

No worries, I'm totally swamped these days myself. It seems everything I touch just turns into more work. Bah :)

[thomasnield]

Haha that's not a bad thing though. Unlimited work is good if it can be paced. Been living and breathing TornadoFX and RxJava at work everyday for the past few months.

But I got around to at least reviewing. Thanks again @t-boom for sweeping these.

Let's ask this: are there any that SHOULD NOT be optional? For the ones I've looked at, it just seems there are times these controls are created simply to be placeholders, even if they don't have lambda bodies to populate and manipulate it. I think with mostof these (correct me if I'm wrong) you can simply create an instance via constructor. Maybe we should allow functional equivalents to do the same.

I've labeled things that I am guessing should have the op argument remain mandatory, as for these there is no useful case where an op argument would be omitted.

Charts.kt

• Pane.piechart()
• CharType.multiseries()

Components.kt

• Component.preferences() MANDATORY

ItemControls.kt

• Pane.tableview()
• Pane.treeview()
• Pane.treetableview()

Layouts.kt

• GridPane.row()
• BorderPane.top()
• BorderPane.bottom()
• BorderPane.left()
• BorderPane.roght()
• BorderPane.center()
• Pane.scrollpane()
• SplitPane.items() MANDATORY
• Accordion.fold()

Libs.kt

• Clipboard.setContent()

Menu.kt

• MenuBar.menu()
• ContextMenu.menu()
• Menu.menu()
• Control.contextmenu()

Nodes.kt

• Toolbar.children() MANDATORY
• children() MANDATORY
• T.borderpaneConstraints() MANDATORY
• T.gridpaneConstraints() MANDATORY
• T.vboxConstraints() MANDATORY
• T.hboxConstraints MANDATORY
• T.anchorpaneConstraints() MANDATORY

[edvin]

Great work guys! I've made these optional except for Clipboard.setContent where it doesn't make sense :)

[thomasnield]

Awesome thanks Edvin!