Bug #28994

General issues from reading the code

Added by Georg Ringer over 8 years ago. Updated about 8 years ago.

Status:
New
Priority:
Should have
Assignee:
-
Target version:
-
Start date:
2011-08-16
Due date:
% Done:

0%


Description

Hi,

as Olly asked in mailinglist for checking the code, here is a short report. I try to keep the CGL infos low, those can be cleaned up laster.

- Directories "Hook" and "Task" should be called "Hooks" and "Tasks" like Classes, Resouces and Tests. Direcotry "Javascript" should be called "JavaScript"
- tx_staticfilecache_Task_AdditionalFieldProvider_DeleteDirtyPagesTaskneeds a hsc() at value="' . $taskInfo['maxEntries'] . '" to avoid XSS. or maybe even an intval if this is a time period
- Classes should begin with Tx_ instead of tx_
- use debug statements for debugging, this makes it easier to find those at the end, e.g. in tx_staticfilecache_Hook_FrontendHook there is an "echo".
- Copyright notice is missing in some files
- Remove not needed files like changelog, readme.txt
- Move icon files into Resources/Public/Images, Move locallang_db.xml into correct directory. xlf?
- ext_conf_template.txt: texts can be localized
- Hooks in ext_localconf.php: I always use instead of "['t3lib/class.t3lib_tcemain.php']['clearPageCacheEval'][]" something like "['t3lib/class.t3lib_tcemain.php']['clearPageCacheEval'][$_EXTKEY]" because it makes it possible to change the hook in a 3rd party extension very easily
- tx_staticfilecache_Hook_FrontendHook: any of the variables covered with t3lib_div::getIndPenv() ?
- just from reading: is the directory "typo3temp/tx_staticfilecache" created if it is missing?
- tx_Staticfilecache_ExtDirect_Server: besides code commenting and phpdocs, I guess TYPO3 got somewhere a translated string of yes/no
- TYPO3 cgl uses a ?> at the end of the file, see ext_tables.php

thanks for your work!

History

#1 Updated by Oliver Hader about 8 years ago

Georg Ringer wrote:

Hi,

as Olly asked in mailinglist for checking the code, here is a short report. I try to keep the CGL infos low, those can be cleaned up laster.

- Directories "Hook" and "Task" should be called "Hooks" and "Tasks" like Classes, Resouces and Tests. Direcotry "Javascript" should be called "JavaScript"

Okay, could be considered...

- tx_staticfilecache_Task_AdditionalFieldProvider_DeleteDirtyPagesTaskneeds a hsc() at value="' . $taskInfo['maxEntries'] . '" to avoid XSS. or maybe even an intval if this is a time period

Obvious, thanks!

- Classes should begin with Tx_ instead of tx_

Ok, since it's based on Extbase - however only the backend module utilizes Extbase, the rest is still "plain" TYPO3...

- use debug statements for debugging, this makes it easier to find those at the end, e.g. in tx_staticfilecache_Hook_FrontendHook there is an "echo".
- Copyright notice is missing in some files
- Remove not needed files like changelog, readme.txt
- Move icon files into Resources/Public/Images, Move locallang_db.xml into correct directory. xlf?
- ext_conf_template.txt: texts can be localized
- Hooks in ext_localconf.php: I always use instead of "['t3lib/class.t3lib_tcemain.php']['clearPageCacheEval'][]" something like "['t3lib/class.t3lib_tcemain.php']['clearPageCacheEval'][$_EXTKEY]" because it makes it possible to change the hook in a 3rd party extension very easily
- tx_staticfilecache_Hook_FrontendHook: any of the variables covered with t3lib_div::getIndPenv() ?

Most of these issues make sense.

- just from reading: is the directory "typo3temp/tx_staticfilecache" created if it is missing?

Yes, it is...

- tx_Staticfilecache_ExtDirect_Server: besides code commenting and phpdocs, I guess TYPO3 got somewhere a translated string of yes/no

Not done yet, however the l10n layer could be introduced here.

- TYPO3 cgl uses a ?> at the end of the file, see ext_tables.php

thanks for your work!

Thanks for your feedback and analysis!

Also available in: Atom PDF