Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce amount done per function #31

Open
edsmall-bodc opened this issue May 26, 2020 · 2 comments
Open

Reduce amount done per function #31

edsmall-bodc opened this issue May 26, 2020 · 2 comments
Labels
ignore-for-release Ignore this for next release refactoring improve the design, structure, and/or implementation

Comments

@edsmall-bodc
Copy link
Collaborator

It goes without saying that this is low priority, for now.

Some functions, such as update_salinity mapping, are doing far too much leg work for a single function. Because of this, they require numerous local variable definitions, and require a huge amount branches (nested for and if statements).

Not only does this make them somewhat difficult to test (as the testing suite to hit all possible branches would be huge), it makes them difficult to maintain for anyone approaching the code without a huge amount of background knowledge.

I propose a reasonably simple, but potentially time consuming refactor whereby we split these routines into even smaller chunks, separating out data manipulation areas (ie converting from 0 < x < 360 latitude to -180 < x < 180) and sections after "if" and "for" statements into their own easily testable functions, if they aren't already.

This will

  1. Allow testing to be even more thorough and effective
  2. Make code reading and navigating far simpler
  3. Make changes easier to implement in the long term
  4. Allow for more accurate and precise documentation
@gmaze
Copy link
Member

gmaze commented May 26, 2020

A more modular approach with class and methods is probably more appropriate here

@gmaze gmaze transferred this issue from euroargodev/User-Acceptance-Test-Python-version-of-the-OWC-tool Jul 7, 2020
@gmaze gmaze added the refactoring improve the design, structure, and/or implementation label Jul 7, 2020
@kamwal
Copy link
Contributor

kamwal commented Apr 28, 2021

The update_salinity_mapping has been refactored to improve the redability of this code.
The refactoring includes:
-moving some of the code out into smaller functions
-reduce the number of variables beeing passed using the dictionaries

More refactoring of other codes associated with update salinity mapping still beed to be done.

@kamwal kamwal added the ignore-for-release Ignore this for next release label Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ignore-for-release Ignore this for next release refactoring improve the design, structure, and/or implementation
Projects
None yet
Development

No branches or pull requests

3 participants