ieugen commented on PR #837: URL: https://github.com/apache/ofbiz-framework/pull/837#issuecomment-2414893653
hi @danwatford , Thanks for taking the time. > so should carefully implement our own IoC patterns per your suggestion. I think this is the sensible way to move forward. > Personally, I'm happy for classes at the same level or in the same module to reference each other The issue is with circular dependencies. It often leads to situations where it is very hard to refactor - like our case now. Especially if lower lever Util classes depend on "higher" level classes like Entity . > In the particular case of UtilCache/UtilProperties/Debug, it seems to me that the configuration is actually in the static initialiser of Debug Yes, this one should be easy to fix via the IoC pattern. > My preference would be to break the link in Debug by removing reference to UtilProperties. Instead there should be a static method in Debug to apply the configuration that is currently being read from debug.properties. Some other class (such as Start) should be responsible for reading debug.properties and determining the debug configuration. Totally agree. > I'm broadly supportive of the pattern, but IoC with static initialisers and methods can be risky. Focussing on parts of the dependency cycle where we can reduce the responsibilities a class has might yield a similar result. Agree. We are where we are and we can do what we can do. Baby steps forward is ok for me. At some point we might have the IoC library/framework discussion. > please consider adding a null check in the places where URL_CACHE is used and throw a suitable exception Sounds reasonable. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@ofbiz.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
