Page MenuHomePhabricator

examples: js: make sure we declare all variables
ClosedPublic

Authored by stefan_schmidt on Dec 16 2019, 5:27 AM.

Details

Summary

Avoiding local variables to be declared global and shared automatically.

Depends on D10882
Reported-By: https://lgtm.com/projects/g/Enlightenment/efl

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
stefan_schmidt created this revision.Dec 16 2019, 5:27 AM

It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/

ProhtMeyhet added inline comments.
src/examples/elementary/twitter_example_01.js
43

I'm not certain about portability, but this example from Mozilla seems to indicate that the variant for (var i = 0; i < 9; i++) { is possible and would make much cleaner code.

71

Instead of explicitly declaring user_icon couldn't it just be var user_icon = layout.part("user_icon").cast("Efl.Content"); here ?

81

Same as above: I'm not certain about portability, but this example from Mozilla seems to indicate that the variant for (var i = 0; i < 9; i++) { is possible and would make much cleaner code.

zmike requested changes to this revision.Dec 23 2019, 6:56 AM
zmike added a subscriber: zmike.

Can confirm explicit inline var declaration in for loops is viable. Since this is example code, let's try to make it look somewhat natural for prospective js users.

This revision now requires changes to proceed.Dec 23 2019, 6:56 AM
stefan_schmidt edited the summary of this revision. (Show Details)

Feedback on inline loop variables

zmike accepted this revision.Dec 24 2019, 6:22 AM
This revision is now accepted and ready to land.Dec 24 2019, 6:22 AM
This revision was automatically updated to reflect the committed changes.