Page MenuHomePhabricator

Turn properties into protected Getters in LanguageConverter based hierarchy
Closed, ResolvedPublic8 Estimated Story Points

Description

Story

When I refactor LanguageConverter I want to avoid overriding constructors by children, so I can easily add/remove new arguments without changing code for all classes in the hierarchy.

Context

At the moment LanguageConverter:: __construct takes too many arguments:

public function __construct( $langobj, $maincode, $variants = [],
							$variantfallbacks = [], $flags = [],
							$manualLevel = [] ) {
...

Most of them (except $langobj) are constant and injected by inheritors:

class EnConverter extends LanguageConverter {

	public function __construct( $langobj ) {
		parent::__construct( $langobj, 'en', [ 'en', 'en-x-piglatin' ] );
	}
...

As a possible solution, the idea is to turn such variables into protected getters overridden by inheritors. This will move a significant part of the logic to that constructors and make it possible not to override it.

Expected Result
  1. Turn $maincode, $variants, $variantfallbacks, $flags, $manualLevel into getters
  2. Override getters in inheritors of LanguageConverter
  3. Avoid overriding the constructor of inheritors when add/remove new argument.

Event Timeline

Peter.ovchyn updated the task description. (Show Details)
Peter.ovchyn updated the task description. (Show Details)
Peter.ovchyn set the point value for this task to 2.

Change 602697 had a related patch set uploaded (by Art.tsymbar; owner: Art.tsymbar):
[mediawiki/core@master] Language: Turn properties into Getters in LanguageConverter based hierarchy

https://gerrit.wikimedia.org/r/602697

From @daniel:

Hey! I just went through the patch. Looks good to me. If we had tests that verify that the conversion still works, we could just merge this...
A proper test suite for the converters would be nice to have, but hard to write without knowing the languages. We could try to just fetch some example pages from each wiki, and turn them into test fixtures...

Art.tsymbar changed the point value for this task from 2 to 8.Nov 19 2020, 2:36 PM

What was done:

  1. Turned $maincode, $variants, $variantfallbacks, $flags, $manualLevel into getters
  2. Overrided getters in inheritors of LanguageConverter
  3. Added new "deprecation" functionality (core\includes\debug\DeprecationHelper.php) and tests for it (core\tests\phpunit\includes\debug\DeprecationHelperTest.php) (almos done)

To be finished:

  1. Resolve merge conflict
  2. Resolve last comments from DannyS712 and Ppchelko

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/602697

Please, write if need any help.

Change 602697 merged by jenkins-bot:
[mediawiki/core@master] Language: Turn public properties into Getters in LanguageConverter based hierarchy

https://gerrit.wikimedia.org/r/602697

Pchelolo claimed this task.