You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
As I cannot comment it on the class above, I'm doing it here how - also not related to this change.
Not for this change, but for a followup - I would suggest to transform this to a shared stateless service instead and let it be injected into the classes needing it.
Furthermore, the checkTableIsAllowedOnPage is using a static property to share a DataHandler instance. This is a completly no-go.
a) Static utility classes are required to be stateless !
b) DataHandler is a stateless class, and should not be shared
So, if the internal state should be reshared, better use a RuntimeCache (can be injected if this class is transformed from a static utility class to a shared service) and cache the $pageId, $tablename result directly and only call the datahandler method (on a new instance) if needed.
So please do me at least the favour to add a proper class todo comment that this class should be refactored to a stateless service and removing the datahandler reusage / property.
Not for this change, but for a followup - I would suggest to transform this to a shared stateless service instead and let it be injected into the classes needing it.
Furthermore, the
checkTableIsAllowedOnPage
is using a static property to share a DataHandler instance. This is a completly no-go.a) Static utility classes are required to be stateless !
b) DataHandler is a stateless class, and should not be shared
So, if the internal state should be reshared, better use a RuntimeCache (can be injected if this class is transformed from a static utility class to a shared service) and cache the $pageId, $tablename result directly and only call the datahandler method (on a new instance) if needed.
So please do me at least the favour to add a proper class todo comment that this class should be refactored to a stateless service and removing the datahandler reusage / property.
Originally posted by @sbuerk in #35 (comment)
The text was updated successfully, but these errors were encountered: