[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