webots icon indicating copy to clipboard operation
webots copied to clipboard

Remove 'global' Variables from Python Code

Open DavidMansolino opened this issue 5 years ago • 4 comments

Describe the Bug Some of our python script are usign the global keyword, this is known as a bad practice (see for example: https://docs.quantifiedcode.com/python-anti-patterns/maintainability/using_the_global_statement.html) and can be avoided in 99% of the cases (personally I have never used it and never felt the need to use it):

  • projects/languages/ros/controllers/ros_python/ros_controller.py
  • resources/web/server/session_server.py
  • resources/web/server/simulation_server.py
  • tests/test_suite.py

DavidMansolino avatar Mar 04 '20 10:03 DavidMansolino

I've just look that up, and there is 3 usage of the global variables : (for instance in resources/web/server/session_server.py)

  • Reference to "constants" like LOAD_THRESHOLD : using global is useless, as global variable/constants could be read from anywhere. moreover sometimes LOAD_THRESHOLD is used without being declared global before (which is fine, but not homogeneous with the rest of the code). Is the intend to emphasis the fact that we are using "global" values ?
  • Reference to object that are being updated, but the reference of the object stay the same : for instance "config" is only updated, hence, as long as we don't change the reference ie config = ..., it's fine to do stuff like config[...] = ... without needing the global keyword
  • Variable needing the global keyword, as availability, modified in update_load. Is that a symptom of bad function definition / usage / architecture ? Should we update it ?

To generalize, are there some unspoken rules about it in the Webot code. Do you thing it should be relevant to spend a bit of time to rearrange the code to make those global disappear ? I should be able to do so. The main question is also about useless global statement to read global variable : removing those global statement will hide the fact that there are global stuff running around. Is that fine ? TBF, it's common in python scripting... And if we don't mess with it (ie don't blindly read/write), it should be fine.

ShuffleWire avatar Jul 08 '22 09:07 ShuffleWire

This is not an important issue and there is a risk to introduce bugs while modifying this code. So, I would prefer that you contribute something else.

omichel avatar Jul 08 '22 09:07 omichel

That's fine, but just for the record, in R2022a, here is the list of python script using the keyword global (and the number of occurrences) scripts/converter/convert_geometries.py (1) projects/robots/mobsya/thymio/controllers/thymio2_aseba/aseba/maintainer/updatedoc/wikidot/structure.py (5) projects/robots/mobsya/thymio/controllers/thymio2_aseba/aseba/maintainer/translations/translation_tools.py (2) projects/default/resources/sumo/tools/traci/main.py (1) tests/test_suite.py (4) src/controller/matlab/mgenerate.py (2) resources/web/server/session_server.py (9) resources/web/server/simulation_server.py (23) resources/webots_ros/scripts/ros_controller.py (1) resources/webots_ros/scripts/ros_python.py (2)

ShuffleWire avatar Jul 08 '22 09:07 ShuffleWire

global variables are still used in webots_ros and webots_server repositories.

stefaniapedrazzi avatar Oct 26 '22 06:10 stefaniapedrazzi