[TYPO3-core] RFC #10011: New feature: TypoScript Condition to check for installed extensions

Oliver Klee typo3-german-02 at oliverklee.de
Thu Sep 30 23:50:46 CEST 2010


Hi Franz,

thanks for the new patch.

Some suggestions:

- Please change )<tab>{ to )<space>{ as per the current CGL.

- The following variable names are still abbreviations or acronyms.
Please rename them:
  - $vDat
  - $rc (several times)
  - $res
  - array key "version_dev"

- For the test case, I recommend to have more than one test. Otherwise,
your new function could just always return TRUE and still pass the test.

- Concerning the function documentation "Returns version information":
Please elaborate. What is "version information", and to what does it
relate? Which return format is to be expected?

As a guideline, the documentation of a function should be so good that
reading it provides all information needed to use the function,
including the format and the boundaries of the parameters and the
formate of the return values.

Please also rework the other function documentations accordingly.

- the function name "makeVersion" and the documentation comment "returns
version information" don't match.

- versionDifference: Please use a function name with a verb in it.

- Please add the missing spaces around the / in the function.

- Please drop the space between the function names and the opening
parenthesis.

- For getExtensionInfo, I'd recommend the use of guard clauses to reduce
the level of nesting.

- In t3lib_matchcondition_abstract, you don't need the require_once as
the autoloader already should take care of this.

- In evaluateConditionCommon, you have about 10 levels of indentation.
This is completely unreadable. This calls for a refactoring using the
"extract method" refactoring.

All the best,


Oli
-- 
Certified TYPO3 Integrator | TYPO3 Security Team Member


More information about the TYPO3-team-core mailing list