{"id":223,"date":"2021-02-04T17:10:55","date_gmt":"2021-02-04T17:10:55","guid":{"rendered":"http:\/\/www.martin-rdz.de\/?p=223"},"modified":"2021-12-23T10:36:03","modified_gmt":"2021-12-23T10:36:03","slug":"micro-refactoring-long-bool-statements","status":"publish","type":"post","link":"http:\/\/www.martin-rdz.de\/index.php\/2021\/02\/04\/micro-refactoring-long-bool-statements\/","title":{"rendered":"micro-refactoring: long bool statements"},"content":{"rendered":"

“Hell is other people’s code” <\/em>– while trying to reprocess some data on our server, a colleagues script did not produce the desired output. The problem was, that the location attribute was missing in the input netCDF files [1]. Which leads us to the code, that checked the attributes:<\/p>\n

#next the formatting I'm supposed to check it against:\r\n\r\n(key_for_range_dimension,key_for_time_dimension,key_for_azimuth_variable,key_for_elevation_variable,key_for_range_variable,key_for_unix_time_variable,key_for_doppler_velocity_variable,key_for_doppler_velocity_error_variable,key_for_nyquist_velocity_variable)=variable_name_key_setting_function(location_we_want,system_we_want,location_info,formatting_translator_dictionary,verbose)\r\n       \r\n#this is a logical statement that includes everything, using brackets to break up inline statements and make it easier to see\r\nformatting_is_correct=(\r\n               'location' in that_files_attributes\r\n               ) and (\r\n                              'system' in that_files_attributes\r\n                              ) and (\r\n                                            key_for_range_dimension in that_files_dimensions\r\n                                            ) and (\r\n                                                           key_for_range_variable in that_files_variables\r\n                                                           ) and (\r\n                                                                         key_for_time_dimension in that_files_dimensions\r\n                                                                          ) and (\r\n                                                                                        key_for_unix_time_variable in that_files_variables\r\n                                                                                        ) and (\r\n                                                                                                        key_for_azimuth_variable in that_files_variables\r\n                                                                                                        ) and (\r\n                                                                                                                       key_for_elevation_variable in that_files_variables\r\n                                                                                                                       ) and (\r\n                                                                                                                                     key_for_doppler_velocity_variable in that_files_variables\r\n                                                                                                                                     )\r\n\r\nprint(\"Check format keys:\")\r\nprint(('location' in that_files_attributes),\r\n    ('system' in that_files_attributes),(key_for_range_dimension in that_files_dimensions),\r\n    (key_for_range_variable in that_files_variables),(key_for_time_dimension in that_files_dimensions),\r\n    (key_for_unix_time_variable in that_files_variables),\r\n    (key_for_azimuth_variable in that_files_variables),\r\n    (key_for_elevation_variable in that_files_variables),\r\n    (key_for_doppler_velocity_variable in that_files_variables))\r\n<\/pre>\n
[You might want to expand the code snippet to fully appreciate the >380 column lines]<\/p>\n
At least for me, that looked like a classical broken<\/a> window<\/a>. The final print statement is already a owned to the debugging efforts. Two points, that I consider problematic:<\/p>\n
    \n
  1. \n
      \n
    1. insanely long lines (not the only occurrence in that script but a prominent one)<\/li>\n
    2. the long and nested boolean statement<\/li>\n<\/ol>\n<\/li>\n<\/ol>\n
      Both make it hard to understand, debug and modify the code. Let’s have see, what we quickly can do about it.<\/p>\n
      var_name_key_setting = variable_name_key_setting_function(\r\n               location_we_want, system_we_want, location_info,\r\n               formatting_translator_dictionary, verbose)\r\n# unpack the function return\r\n(key_for_range_dimension, key_for_time_dimension, \r\n key_for_azimuth_variable, key_for_elevation_variable, \r\n key_for_range_variable, key_for_unix_time_variable, \r\n key_for_doppler_velocity_variable, \r\n key_for_doppler_velocity_error_variable,\r\n key_for_nyquist_velocity_variable) = var_name_key_setting\r\n       \r\nformatting_is_correct_list = [\r\n    'location' in that_files_attributes, \r\n    'location in attributes',\r\n    'system' in that_files_attributes, \r\n    'system in attributes',\r\n    key_for_range_dimension in that_files_dimensions, \r\n    'key range dim in files_dimensions',\r\n    key_for_range_variable in that_files_variables,\r\n    'key range var in files_variables',\r\n    key_for_time_dimension in that_files_dimensions,\r\n    'key time dim in files_dimensions',\r\n    key_for_unix_time_variable in that_files_variables,\r\n    'key unix_time_var in files_variables',\r\n    key_for_azimuth_variable in that_files_variables,\r\n    'key azimuth var in files_variables',\r\n    key_for_elevation_variable in that_files_variables,\r\n    'key elevation var in files_variables',\r\n    key_for_doppler_velocity_variable in that_files_variables,\r\n    'key doppler_vel_var in files_variables',\r\n       ]\r\nformatting_is_correct = all(formatting_is_correct_list[::2])\r\n\r\nprint(\"Check format keys:\", formatting_is_correct)\r\nif not formatting_is_correct:\r\n       [print('  ', e[0], e[1]) for e in \\\r\n            zip(formatting_is_correct_list[0:-1:2], \r\n                formatting_is_correct_list[1::2])]\r\n<\/pre>\n
      Still not perfect, but a bit more explicit and easier to read. In fact the [::2]<\/code> could also be omitted, as a string also evaluates to True in all()<\/code>.<\/p>\n
      Maybe it’s time to setup a compulsory linter or at least make PEP 8<\/a> and the Google Python Style Guide<\/a> required reading.<\/p>\n
      [1] actually required a second pair of eyes to figure out…<\/p>\n","protected":false},"excerpt":{"rendered":"
      “Hell is other people’s code” – while trying to reprocess some data on our server, a colleagues script did not produce the desired output. The problem was, that the location attribute was missing in the input netCDF files [1]. Which leads us to the code, that checked the attributes: #next the formatting I’m supposed to …<\/p>\n","protected":false},"author":1,"featured_media":0,"comment_status":"closed","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[7],"tags":[],"jetpack_sharing_enabled":true,"jetpack_featured_media_url":"","_links":{"self":[{"href":"http:\/\/www.martin-rdz.de\/index.php\/wp-json\/wp\/v2\/posts\/223"}],"collection":[{"href":"http:\/\/www.martin-rdz.de\/index.php\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"http:\/\/www.martin-rdz.de\/index.php\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"http:\/\/www.martin-rdz.de\/index.php\/wp-json\/wp\/v2\/users\/1"}],"replies":[{"embeddable":true,"href":"http:\/\/www.martin-rdz.de\/index.php\/wp-json\/wp\/v2\/comments?post=223"}],"version-history":[{"count":9,"href":"http:\/\/www.martin-rdz.de\/index.php\/wp-json\/wp\/v2\/posts\/223\/revisions"}],"predecessor-version":[{"id":271,"href":"http:\/\/www.martin-rdz.de\/index.php\/wp-json\/wp\/v2\/posts\/223\/revisions\/271"}],"wp:attachment":[{"href":"http:\/\/www.martin-rdz.de\/index.php\/wp-json\/wp\/v2\/media?parent=223"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"http:\/\/www.martin-rdz.de\/index.php\/wp-json\/wp\/v2\/categories?post=223"},{"taxonomy":"post_tag","embeddable":true,"href":"http:\/\/www.martin-rdz.de\/index.php\/wp-json\/wp\/v2\/tags?post=223"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}