ImperialCollegeLondon / virtual_ecosystem

This repository is the home for the codebase for the Virtual Ecosystem project.
https://virtual-ecosystem.readthedocs.io
BSD 3-Clause "New" or "Revised" License
9 stars 1 forks source link

simplify calculate_soil_carbon_updates() function #344

Open jacobcook1995 opened 10 months ago

jacobcook1995 commented 10 months ago

Is your feature request related to a problem? Please describe. The function to calculate the updates for the soil carbon pool (calculate_soil_carbon_updates) is much too complex, it's 150 lines long and has 17 arguments.

Describe the solution you'd like I should remember to simplify the function. There should hopefully be an obvious moment to do this at some point when I'm adding more stuff to the soil model.

@alexdewar also suggested the following as good generic approaches to this sort of problem:

  1. If lots of the arguments to this function are just being passed through to one or two other functions, you could just pass in the results of these functions instead and have the caller calculate them
  2. If some of the arguments would naturally group together, consider bundling them together in a class/dataclass (provided that this function won't be the only user of it)
jacobcook1995 commented 3 months ago

@davidorme had some a helpful suggestions here, which I should bear in mind for the next time I need to simplify this function