[TYPO3-core] [FEATURE] Flexible cHash Calculation
Helmut Hummel
helmut.hummel at typo3.org
Mon Feb 20 21:55:57 CET 2012
Hi,
On 20.02.12 19:28, Tolleiv Nietsch wrote:
> The patchset wasn't intended to introduce a new public API or and kind
> of class anyone could use from the outside of the TYPO3 Core. Also
> changes "during" runtime aren't really efficient anyhow, so providing
> public setters and getters isn't really relevant outside of the tests.
I did not introduce any getters. I'm fine changing the respective
setters to protected and using an accessibleMock in the tests.
I also thought of adding the setter with @private which is also used in
some cases where the public visibility was only for technical reasons.
However making the API clean even if it's only for internal use and
doing it in a way which is easier/ straightforward to test makes
absolutely sense to me.
Adding new classes which introduce hard wired dependencies to global
variables is bad style imho. We should at least try to make the
dependiencies "injectable" or at least sort of.
> Thats why I'd vote against Helmut's last patchset in favor of patchset
> 12 or 13. Once again I'd kindly ask for comments and reviews for the
> patchsets in general.
As of Daniel's comments:
> I see the following cons for the last change in t3lib_cacheHash
> 1) Its not possible to configure cacheHash from Installtool anymore
Wrong. It's still possible. However I doubt the usefulness of that
especially if we introduce awkward API because of a limitation in the
install tool code which is really old and to be replaced.
> 2) the set* methods are not used in the core and therefore introduce
> unrequired public methods to the class interface
I'm fine with changing that.
> 3) the setConfiguration uses method_exist check - even if its just
> nanoseconds i think hardcoded configuration keys are also ok.
Introducing a new object costs a lot more that nanoseconds. If we want
to save execution time an memory we should probably leave everything
like it is. Right? No!
These calls are for safety only and could easily be skipped, if you
really think it is worth it.
> So for the moment i vote for the previous patchset
I would have voted -1 for the previous patchset, but I invested 2h to
comply to the patch for a patch rule. Does not seem to pay off.
Kind regards,
Helmut
--
Helmut Hummel
TYPO3 Security Team Leader, TYPO3 v4 Core Team Member
TYPO3 .... inspiring people to share!
Get involved: typo3.org
More information about the TYPO3-team-core
mailing list