[TYPO3-core] RFC: #16315: New extension manager part 1: splitting classes
Xavier Perseguers
typo3 at perseguers.ch
Wed Nov 10 12:01:31 CET 2010
Hi Steffen,
Here's my review by reading for the first ~ 2200 lines. Both of you made
a crazy job!
+ /**
+ * Get extension list from cache_extensions
+ *
+ * @param $repository
^^^^ missing type
+ * @param string $andWhere
+ * @param string $orderBy
+ * @param string $orderDir
+ * @param string $limit
+ * @return array
+ */
+ public function getExtensionListFromRepository($repository, $andWhere
= '', $orderBy = '', $orderDir = 'ASC', $limit = '') {
+ $order = $orderBy ? $orderBy . ' ' . $orderDir : '';
+ $ret = array();
+
+ $temp = $GLOBALS['TYPO3_DB']->exec_SELECTgetRows(
+ 'count(*) count',
+ 'cache_extensions',
+ 'repository=' . intval($repository) . $andWhere,
+ 'extkey'
+ );
+ $ret['count'] = count($temp);
^^^^ It's a pity exec_SELECTcountRows cannot take a "group by" argument
+ $ret['results'] = $GLOBALS['TYPO3_DB']->exec_SELECTgetRows(
+ '*, count(*) versions, max(intversion) maxintversion',
+ 'cache_extensions',
+ 'repository=' . intval($repository) . $andWhere,
+ 'extkey',
+ $order,
+ $limit
+ );
+ return $ret;
+ }
+
For aliases in field names, we don't have CGL but personally I prefer
having the explicit keyword "AS"
+ /**
+ * Get versions of extension
+ *
+ * @param int $repository
+ * @param string $extKey
+ * @retuen array $versions
+ */
+ public function getExtensionVersionsFromRepository($repository, $extKey) {
@retuen instead of @return
+ /**
+ * Function inserts a repository object into database.
+ *
+ * @access public
+ * @param tx_em_Repository $repository repository object
+ * @return void
+ */
+ public function updateRepository(tx_em_Repository $repository) {
Strange description! Copy paste from next:
+ /**
+ * Function inserts a repository object into database.
+ *
+ * @access public
+ * @param tx_em_Repository $repository repository object
+ * @return integer UID of the newly inserted repository object
+ */
+ public function insertRepository(tx_em_Repository $repository) {
At least for these 2 methods, you start with "Function <some verb>",
why? Later you have multiple times "Method <some verb>"... I'd prefer
having simply the description starts with the verb in the 3rd person
each time.
+ function dumpTableHeader($table, $fieldKeyInfo, $dropTableIfExists = 0) {
+ $lines = array();
+ $dump = '';
+
+ // Create field definitions
+ if (is_array($fieldKeyInfo['fields'])) {
+ foreach ($fieldKeyInfo['fields'] as $fieldN => $data) {
+ $lines[] = ' ' . $fieldN . ' ' . $data;
+ }
+ }
+
+ // Create index key definitions
+ if (is_array($fieldKeyInfo['keys'])) {
+ foreach ($fieldKeyInfo['keys'] as $fieldN => $data) {
+ $lines[] = ' ' . $data;
+ }
+ }
I know it was borrowed, but I don't like having "$fieldN" as key
variable for the loop.
+ /**
+ * Enter description here...
+ *
+ * @return unknown
+ */
+ public function getInstalledExtkeys() {
Missing description
+ /**
+ * Keeps sponsor link of currently processed mirror.
+ *
+ * @var string
+ */
+ protected $sponsorlink = NULL;
+
+ /**
+ * Keeps sponsor logo location of currently processed mirror.
+ *
+ * @var string
+ */
+ protected $sponsorlogo = NULL;
+
+ /**
+ * Keeps sponsor name of currently processed mirror.
+ *
+ * @var string
+ */
+ protected $sponsorname = NULL;
What about lower camel case?
+ /**
+ * Returns an assoziative array of all mirror properties.
+ *
+ * Valid array keys of returned array are:
+ * country, host, path, sponsorlink, sponsorlogo, sponsorname, title
+ *
+ * @access public
+ * @return array assoziative array of a mirror's properties
+ * @see $country, $host, $path, $sponsorlink, $sponsorlogo,
$sponsorname, $title
+ */
+ public function getAll() {
The "z" of "assoziative" sounds really german to me ;-)
+/**
+ * Importer object for extension list
+ *
+ * @author Marcus Krause <marcus#exp2010 at t3sec.info>
+ * @author Steffen Kamper <info at sk-typo3.de>
+ *
+ * @since 2010-02-10
+ * @package TYPO3
+ * @subpackage EM
+ */
+class tx_em_Import_ExtensionListImporter implements SplObserver {
+
Shouldn't be @since a TYPO3 version instead? Anyway, the date is wrong.
+ /**
+ * Keeps fieldnames of cache_extension table.
+ *
+ * @var array
+ */
+ static protected $fieldNames = array('extkey', 'version',
'intversion', 'alldownloadcounter', 'downloadcounter', 'title',
'ownerusername', 'authorname', 'authoremail', 'authorcompany',
'lastuploaddate', 't3xfilemd5', 'repository', 'state', 'reviewstate',
'category', 'description', 'dependencies', 'uploadcomment' /*,
'lastversion', 'lastreviewedversion'*/);
+
Could this huge list be split and stored as multiple lines?
+ /**
+ * Keeps indexes of fields that should not be quoted.
+ *
+ * @var array
+ */
+ static protected $fieldIndicesNoQuote = array(2, 3, 4, 10, 12, 13, 14,
15);
This typically is a list of "magic numbers"...
What about using prepared queries?
Cheers
Xavier
More information about the TYPO3-team-core
mailing list