Task #63326
closed
Avoid using $GLOBALS['TYPO3_DB'] in DataHandler
100%
Description
Instead of $GLOBALS['TYPO3_DB'] a type hinted property $this->databaseConnection should be used to gain autocompletion and better IDE inspection in this class.
Concerns: This might be a breaking change. $this->databaseConnection is set on DataHandler->start(). If some code relies on setting/modifying $GLOBALS['TYPO3_DB'] after DataHandler->start() and before other methods of DataHandler are invoked it might break. Please decide if this is too much of a risk.
Updated by Gerrit Code Review almost 10 years ago
- Status changed from New to Under Review
Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/34614
Updated by Mathias Schreiber almost 10 years ago
Absolutely not too risky:
Here's some info on this kind of cleanups:
- Please use imports on top of the class
- Please supply constructor injection as long as it makes sense (*)
- Please supply a meaningful comment on the property
The thing with "where is makes sense".
Your constructor might get a little big over time.
Helmut and I came up with the plan that 3-4 parameters are fine within the constructor DI.
If you should need more than that and have no clue how to deal with it, ping the #typo3-cms channel on slack and we'll find a solution.
These cleanups are a lot more important than you might think.
For starters, they supply code completion in an IDE.
In PHPStorm in particular, it makes even more sense, because all calls to $GLOBALS[SOMETHING] are marked as a warning.
While the warning in itself isn't an issue, the amount of warnings results in a sort of "I don't care" mentality and "real" warnings simply get lost.
So, by all means... YES, PLEASE! Keep fixes like these coming :)
Updated by Gerrit Code Review almost 10 years ago
Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/34614
Updated by Gerrit Code Review almost 10 years ago
Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/34614
Updated by Anonymous almost 10 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset fea7b36dbfd2957cc076ad247594d5f52015c2f1.
Updated by Gerrit Code Review almost 10 years ago
- Status changed from Resolved to Under Review
Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/34630
Updated by Gerrit Code Review almost 10 years ago
Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/34630
Updated by Anonymous almost 10 years ago
- Status changed from Under Review to Resolved
Applied in changeset c2c1fc6936c2d0ba27b63f981f276b7fec13ea4b.
Updated by