search

allfed / allfed-integrated-model

Integrated model to calculate the effects of resilient foods in catastrophic events
https://allfed.github.io/allfed-integrated-model/
GNU General Public License v3.0
11 stars 10 forks source link

Food conversion should not be static #165

Open lukaszgajewski opened 2 weeks ago

lukaszgajewski commented 2 weeks ago

This should not be a static class attribute. Generally we should avoid statics because they are difficult to trace, debug and control. Additionally, we already inherit from UnitConversions so this should have been handled in the constructor instead. Moreover, in cases like this it creates an implicit requirement for the Food class to had been instantiated somewhere else. This is messy and very bug-prone and we should avoid implicit class inter-dependencies. I.e., CellulosicSugar looks like it only needs constants_for_params to be constructed but that's not true, so if I have a constants_for_params dict and I try to do cs = CellulosicSugar(constants_for_params), it will fail because it also depends on Food(). And not only for it to have already been existing but also having had executed several other functions -- this makes it very difficult to use, test and maintain such code without arcane knowledge of the codebase.

morganrivers commented 1 week ago

Agreed, it's confusing. IIRC the reason for this is that you can't convert between some of the units before knowing the global population. So like % fed is dependent on population, but billions of kcals is not.