The option page initializes the GUI elements in a big switch statement in setupControl and has some helper variable in each case of the switch.
Since these helper variables are only valid for the specific case, they should become inaccessible to the other cases of the switch statement to prevent accidental overwrites and
thereby making it quicker to review.
Details
Details
- Reviewers
Polakrity
Convince yourself that these variables changed are only referenced in that specific switch case.
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 1768 Build 2799: Vulcan Build Jenkins Build 2798: arc lint + arc unit
Event Timeline
Comment Actions
Refs D402.
Notice the style is in line with https://trac.wildfiregames.com/wiki/Coding_Conventions
Comment Actions
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/1232/ for more details.
Comment Actions
Moving the config value out of the switch isn't really nice, since in one case we convert it to boolean, in others to a number, so nothing really gained. The idea was to remove occurances out of the inner scope as far as possible.
Following this, I ended up rewriting the entire file :(
Patch:
options.js:
Would like to know if there is opposition to the changes before spending more time on it. Won't be trivial to review.
Comment Actions
This rewriting seems cleaner/better coding style than the actual code.
Object literals increase readability and maintainability in this case.
Maybe edit the title of revision and add the proposal patch in summary for get more reviews.
Comment Actions
Thanks for the feedback. The only drawback of removing the loops is not checking for unknown properties. On the other hand we don't have validation for all the other JSON files we have around. I won't have time to focus on this before the next release though (really low priority since there is no real gain besides a bit of readability)