danwatford commented on PR #837:
URL: https://github.com/apache/ofbiz-framework/pull/837#issuecomment-2413993031
Hi @ieugen ,
I'm generally on-board with the IoC approach, but it is safer when dealing
with instantiated objects rather than static fields in classes. If we had an
IoC framework, such as Spring, I think this would be an easy pattern to
implement.
Alas, we are don't have such a framework, and we shouldn't let 'perfect be
the enemy of good', so should carefully implement our own IoC patterns per your
suggestion.
Personally, I'm happy for classes at the same level or in the same module to
reference each other, as long as those classes do not require any
configuration. In the particular case of UtilCache/UtilProperties/Debug, it
seems to me that the configuration is actually in the static initialiser of
Debug where we find a reference to UtilProperties.
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.
Debug currently has two jobs: reading its configuration from a properties
file; carry out logging. The above proposed changes would reduce Debug's
responsibility and break the dependency cycle between UtilCache, UtilProperties
and Debug.
Summary: 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.
If you decide to go with your original proposal of
` UtilCache<String, Properties> URL_CACHE =
UtilCache.createUtilCache("properties.UtilPropertiesUrlCache");
UtilProperties.setUrlCache(URL_CACHE); `
please consider adding a null check in the places where URL_CACHE is used
and throw a suitable exception to explain the cause, rather than letting a NPE
bubble up the stack.
--
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
