Task #63326

Avoid using $GLOBALS['TYPO3_DB'] in DataHandler

Added by Sebastian Michaelsen over 6 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
DataHandler aka TCEmain
Target version:
-
Start date:
2014-11-25
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
7
PHP Version:
Tags:
Complexity:
Sprint Focus:

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.

#1

Updated by Gerrit Code Review over 6 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

#2

Updated by Mathias Schreiber over 6 years ago

Absolutely not too risky:
Here's some info on this kind of cleanups:

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 :)

#3

Updated by Gerrit Code Review over 6 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

#4

Updated by Gerrit Code Review over 6 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

#5

Updated by Anonymous over 6 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
#6

Updated by Gerrit Code Review over 6 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

#7

Updated by Gerrit Code Review over 6 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

#8

Updated by Anonymous over 6 years ago

  • Status changed from Under Review to Resolved
#9

Updated by Benni Mack over 2 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF