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
checkTableIsAllowedOnPageis 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 https://github.com/sudhaus7/typo3-xlsimport/pull/35#discussion_r1451819150_