Task #63326

Avoid using $GLOBALS['TYPO3_DB'] in DataHandler

Added by Sebastian Michaelsen about 5 years ago. Updated about 1 year ago.

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

100%

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.

Associated revisions

Revision fea7b36d (diff)
Added by Sebastian Michaelsen about 5 years ago

[TASK] Avoid using $GLOBALS['TYPO3_DB'] in DataHandler

Instead of $GLOBALS['TYPO3_DB'] a type hinted property
$this->databaseConnection is used to gain autocompletion and better IDE
inspection in this class.

Resolves: #63326
Releases: master
Change-Id: I97a7158fd7a19ecb9687e66906392678d53b58f6
Reviewed-on: http://review.typo3.org/34614
Reviewed-by: Markus Klein <>
Tested-by: Markus Klein <>
Reviewed-by: Alexander Opitz <>
Tested-by: Alexander Opitz <>

Revision c2c1fc69 (diff)
Added by Sebastian Michaelsen about 5 years ago

Followup: [TASK] Avoid using $GLOBALS['TYPO3_DB'] in DataHandler

  • Fix wrong PHP namespace when importing DatabaseConnection in the test
  • Remove the constructor injection which was added but not used

Resolves: #63326
Releases: master
Change-Id: I310247056647d2ec21f88c606c12220061b6b54b
Reviewed-on: http://review.typo3.org/34630
Reviewed-by: Markus Klein <>
Tested-by: Markus Klein <>
Reviewed-by: Stefan Neufeind <>
Tested-by: Stefan Neufeind <>

History

#1 Updated by Gerrit Code Review about 5 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 about 5 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 about 5 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 about 5 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 about 5 years ago

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

#6 Updated by Gerrit Code Review about 5 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 about 5 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 about 5 years ago

  • Status changed from Under Review to Resolved

#9 Updated by Benni Mack about 1 year ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF