I was doing a bit of work today reviewing the Popup class we’re working on for the next release of JavaFX. I showed Jasper some code snippets and he pointed out that I should write a quick blog on it because it shows a nice progression of how to do looping in JavaFX.
The code in question is relatively easy to understand. The Popup class had a private sequence of “child” popups var childrenPopups:Popup[]. The function is called “hideAll” and will hide each child popup. The original function used a java-like looping mechanism:
[jfx]
public function hideAll():Void {
for ( i in [0..sizeof this.childrenPopups] ) {
if (this.childrenPopups[i].visible) {
this.childrenPopups[i].hideAll();
}
}
hide();
}
[/jfx]
The function is relatively straightforward. We iterate from 0 to sizeof childrenPopups. Except, oops, sequences are inclusive so this actually iterates one time too often! So either we’d need to throw a -1 on there ([0..sizeof this.childrenPopups - 1]), or use the less-than operator ([0..<sizeof this.childrenPopups]).
In any case, we iterate over all the child popups and hide them, accessing them by index. This is fine, and a very java-centric way to do it. However, we can clean up some of the visual noise here by using JavaFX’s built in sequence looping instead:
[jfx]
public function hideAll():Void {
for (p in childrenPopups) {
if (p.visible) p.hideAll();
}
hide();
}
[/jfx]
This is a fair amount cleaner from a visual-clutter perspective. Performance (and indeed, the generated code) is exactly the same as before. One positive gain here is that we avoided the nasty hidden bug in the previous code of iterating one to many times.
I would consider this a completely fx-like implementation of the function. But we can actually take it one step further. In JavaFX looping supports a “where” clause. The performance (and indeed, the generated code!) is exactly the same as with both previous examples. Its mostly just a style thing:
[jfx]
public function hideAll():Void {
for (p in childrenPopups where p.visible) {
p.hideAll();
}
hide();
}
[/jfx]
As mentioned, all three of these implementations essentially generate the exact same code. They all turn into very efficient for loops (unlike the java for-each code which creates iterators in many cases). They all have the same if clause inside the loop and do the same work. It’s just matter of taste and visual clutter.
I think the third option is significantly better than the first option.
And it shows the elegance of jfx script nicely.
@sherod – me too
I agree with Steven. The 3rd option is much nicer.
Rich,
I agree with the others. The third option is cleaner. JavaFX’s ‘where’ keyword makes me think of database SQL statements.
I’m still getting into the JavaFX Script Language mindset (JavaFX-isms, JavaFX-esque). I still have recent code that looks like option one.
-Carl
even, simpler version:
public function hideAll():Void {
for (p in childrenPopups where p.visible) p.hideAll();
hide();
}
And this is in fact what the current code looks like. However to be honest I think all of the last 3 suggestions (pretty well everything but the first) have the same level of simplicity and clarity, just a matter of taste.
Thx for this post. I didn’t know that we can use “where” in loops.
We could also do the same using :
for (p in childrenPopups[child | child.visible]) {
p.hideAll();
}
Regards
Michał
As an alternative for fun, and probably lots of problems with this idea, but if popups know their parent popup, what about something like:
var visControl bind parentPopup.visible on replace {
if (! visControl) then visible = false;
}
I realise somewhat less efficient and would require the parent maintenance which may go against the grain, but then simply setting the parent visibility false would be all that was required 🙂
yeah, but having in mind performance issue when using binding i don’t think that it is worth it.