D28666: Refactor for loops

[Kai Uwe Broulik]

broulik added a comment.


  +1 for the `.some()` use
  -1 on the convoluted `Array.prototype` things, especially when chained.

INLINE COMMENTS

> Action.qml:156
>      readonly property var visibleChildren: {
> -        var visible = [];
> -        var child;
> -        for (var i in children) {
> -            child = children[i];
> -            if (!child.hasOwnProperty("visible") || child.visible) {
> -                visible.push(child)
> -            }
> -        }
> -        return visible;
> +        return Array.prototype.filter.call(children, child => !child.hasOwnProperty("visible") || child.visible)
>      }

Why are you going through the prototype? Or is `children` not a "proper" Array?
Also, can we use spread operator `...` here?
And yes, please make those a bit more readable by using useful line breaks and using parentheses and braces.

> FormLayout.qml:85
>              var hint = 0;
> -            for (var i in knownItems) {
> -                hint = Math.max(hint, knownItems[i].Layout.preferredWidth > 0 ? knownItems[i].Layout.preferredWidth : knownItems[i].implicitWidth);
> -            }
> +            knownItems.forEach(item => hint = Math.max(hint, item.Layout.preferredWidth > 0 ? item.Layout.preferredWidth : item.implicitWidth));
>              return hint;

Looks like a usecas for `Array.reduce`?

> PassiveNotification.qml:62
>  
> -        for (let i = 0; i < outerLayout.children.length - 3; ++i) {
> -            outerLayout.children[i].close();
> -        }
> +        outerLayout.children.slice(0, -3).forEach(child => child.close());
>  

I would like a comment here. Note veryone is faimilar with `slice` in that fashion.

> PassiveNotification.qml:72
>          // Reorder items to have the last on top
> -        let children = outerLayout.children;
> -        for (let i in children) {
> -            children[i].Layout.row = children.length-1-i;
> -        }
> +        outerLayout.children.forEach(function(value, index) => {
> +            value.Layout.row = outerLayout.children.length-1-index;

Why no arrow function?

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D28666

To: cblack, #kirigami
Cc: broulik, ngraham, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, apol, ahiemstra, davidedmundson, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200408/0f35d043/attachment-0001.html>