jenkins-bot has submitted this change and it was merged.

Change subject: Refactor and simplify time formatter classes
......................................................................


Refactor and simplify time formatter classes

This is pure refactoring that does not change any behavior. The goal
is to make following patches easier to review.

* Make $options optional in 2 constructors.
* Drop duplicate code from MwTimeIsoFormatter constructor.
* Drop not needed class variable from TimeDetailsFormatter.
* Simplify TimeDetailsFormatter constructor a lot (all this hasOption
  code is exactly what defaultOption does).
* Move calendar model formatting to private method.
* Drop not needed options from tests. English is the default anyway.
* Use constants instead of "10" in tests.
* Split long lines.

Change-Id: Iab1782a7fe1193cef2c186a9d4a2a2d6a4fa3a6d
---
M lib/includes/formatters/MwTimeIsoFormatter.php
M lib/includes/formatters/TimeDetailsFormatter.php
M lib/tests/phpunit/formatters/TimeDetailsFormatterTest.php
3 files changed, 60 insertions(+), 52 deletions(-)

Approvals:
  Daniel Kinzler: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/lib/includes/formatters/MwTimeIsoFormatter.php 
b/lib/includes/formatters/MwTimeIsoFormatter.php
index 449abe5..5e3524d 100644
--- a/lib/includes/formatters/MwTimeIsoFormatter.php
+++ b/lib/includes/formatters/MwTimeIsoFormatter.php
@@ -28,12 +28,10 @@
        private $language;
 
        /**
-        * @param FormatterOptions $options
+        * @param FormatterOptions|null $options
         */
-       public function __construct( FormatterOptions $options ) {
-               $this->options = $options;
-
-               $this->options->defaultOption( ValueFormatter::OPT_LANG, 'en' );
+       public function __construct( FormatterOptions $options = null ) {
+               parent::__construct( $options );
 
                $this->language = Language::factory(
                        $this->options->getOption( ValueFormatter::OPT_LANG )
diff --git a/lib/includes/formatters/TimeDetailsFormatter.php 
b/lib/includes/formatters/TimeDetailsFormatter.php
index 008d37b..20fb536 100644
--- a/lib/includes/formatters/TimeDetailsFormatter.php
+++ b/lib/includes/formatters/TimeDetailsFormatter.php
@@ -24,34 +24,27 @@
        const OPT_CALENDARNAMES = 'calendars';
 
        /**
-        * @var MwTimeIsoFormatter
-        */
-       private $isoTimeFormatter;
-
-       /**
         * @var TimeFormatter
         */
        private $timeFormatter;
 
        /**
-        * @param FormatterOptions $options
+        * @param FormatterOptions|null $options
         */
-       public function __construct( FormatterOptions $options ) {
+       public function __construct( FormatterOptions $options = null ) {
                parent::__construct( $options );
 
-               if ( $options->hasOption( TimeFormatter::OPT_TIME_ISO_FORMATTER 
) ) {
-                       $this->isoTimeFormatter = $options->getOption( 
TimeFormatter::OPT_TIME_ISO_FORMATTER );
-               } else {
-                       $this->isoTimeFormatter = new MwTimeIsoFormatter( 
$options );
-                       $options->setOption( 
TimeFormatter::OPT_TIME_ISO_FORMATTER, $this->isoTimeFormatter );
-               }
-
-               $this->timeFormatter = new TimeFormatter( $options );
+               $this->defaultOption(
+                       TimeFormatter::OPT_TIME_ISO_FORMATTER,
+                       new MwTimeIsoFormatter( $this->options )
+               );
 
                $this->defaultOption( self::OPT_CALENDARNAMES, array(
                        TimeFormatter::CALENDAR_GREGORIAN => 'Gregorian',
                        TimeFormatter::CALENDAR_JULIAN => 'Julian',
                ) );
+
+               $this->timeFormatter = new TimeFormatter( $this->options );
        }
 
        /**
@@ -70,34 +63,39 @@
                        throw new InvalidArgumentException( 'Data value type 
mismatch. Expected an TimeValue.' );
                }
 
-               $calendarModel = $value->getCalendarModel();
-               $calendarNames = $this->getOption( self::OPT_CALENDARNAMES );
-               if ( array_key_exists( $calendarModel, $calendarNames ) ) {
-                       $calendarModel = $calendarNames[$calendarModel];
-               }
-
                $html = '';
-               $html .= Html::element( 'h4',
+               $html .= Html::element(
+                       'h4',
                        array( 'class' => 'wb-details wb-time-details 
wb-time-rendered' ),
                        $this->timeFormatter->format( $value )
                );
+               $html .= Html::openElement( 'table', array( 'class' => 
'wb-details wb-time-details' ) );
 
-               $html .= Html::openElement( 'table',
-                       array( 'class' => 'wb-details wb-time-details' ) );
-               $html .= $this->renderLabelValuePair( 'isotime', 
htmlspecialchars( $value->getTime() ) );
-
-               //TODO: provide "nice" rendering of timezone, calendar, 
precision, etc.
+               $html .= $this->renderLabelValuePair(
+                       'isotime',
+                       htmlspecialchars( $value->getTime() )
+               );
                $html .= $this->renderLabelValuePair(
                        'timezone',
-                       $this->formatTimezone( $value->getTimezone() )
+                       $this->getTimezoneHtml( $value->getTimezone() )
                );
-               $html .= $this->renderLabelValuePair( 'calendar',
-                       htmlspecialchars( $calendarModel ) );
-               $html .= $this->renderLabelValuePair( 'precision',
-                       htmlspecialchars( $value->getPrecision() ) );
-
-               $html .= $this->renderLabelValuePair( 'before', 
htmlspecialchars( $value->getBefore() ) );
-               $html .= $this->renderLabelValuePair( 'after', 
htmlspecialchars( $value->getAfter() ) );
+               $html .= $this->renderLabelValuePair(
+                       'calendar',
+                       $this->getCalendarModelHtml( $value->getCalendarModel() 
)
+               );
+               // TODO: Provide "nice" rendering of precision, etc.
+               $html .= $this->renderLabelValuePair(
+                       'precision',
+                       htmlspecialchars( $value->getPrecision() )
+               );
+               $html .= $this->renderLabelValuePair(
+                       'before',
+                       htmlspecialchars( $value->getBefore() )
+               );
+               $html .= $this->renderLabelValuePair(
+                       'after',
+                       htmlspecialchars( $value->getAfter() )
+               );
 
                $html .= Html::closeElement( 'table' );
 
@@ -109,7 +107,7 @@
         *
         * @return string
         */
-       private function formatTimezone( $timezone ) {
+       private function getTimezoneHtml( $timezone ) {
                $sign = $timezone < 0 ? "\xE2\x88\x92" : '+';
                $hour = floor( abs( $timezone ) / 60 );
                $minute = abs( $timezone ) - $hour * 60;
@@ -117,6 +115,20 @@
        }
 
        /**
+        * @param string $calendarModel
+        *
+        * @return string
+        */
+       private function getCalendarModelHtml( $calendarModel ) {
+               $calendarNames = $this->getOption( self::OPT_CALENDARNAMES );
+               if ( array_key_exists( $calendarModel, $calendarNames ) ) {
+                       $calendarModel = $calendarNames[$calendarModel];
+               }
+
+               return htmlspecialchars( $calendarModel );
+       }
+
+       /**
         * @param string $fieldName
         * @param string $valueHtml
         *
diff --git a/lib/tests/phpunit/formatters/TimeDetailsFormatterTest.php 
b/lib/tests/phpunit/formatters/TimeDetailsFormatterTest.php
index 9a350d5..144e39a 100644
--- a/lib/tests/phpunit/formatters/TimeDetailsFormatterTest.php
+++ b/lib/tests/phpunit/formatters/TimeDetailsFormatterTest.php
@@ -24,23 +24,23 @@
 
        /**
         * @dataProvider quantityFormatProvider
+        * @param TimeValue $value
+        * @param string $pattern
         */
-       public function testFormat( $value, $options, $pattern ) {
-               $formatter = new TimeDetailsFormatter( $options );
+       public function testFormat( TimeValue $value, $pattern ) {
+               $formatter = new TimeDetailsFormatter( new FormatterOptions() );
 
                $html = $formatter->format( $value );
                $this->assertRegExp( $pattern, $html );
        }
 
        public function quantityFormatProvider() {
-               $options = new FormatterOptions( array(
-                       ValueFormatter::OPT_LANG => 'en'
-               ) );
+               $gregorian = TimeFormatter::CALENDAR_GREGORIAN;
+               $day = TimeValue::PRECISION_DAY;
 
                return array(
                        array(
-                               new TimeValue( '+2001-01-01T00:00:00Z', 60, 0, 
1, 10, TimeFormatter::CALENDAR_GREGORIAN ),
-                               $options,
+                               new TimeValue( '+2001-01-01T00:00:00Z', 60, 0, 
1, TimeValue::PRECISION_MONTH, $gregorian ),
                                '@' . implode( '.*',
                                        array(
                                                
'<h4[^<>]*>[^<>]*2001[^<>]*</h4>',
@@ -54,13 +54,11 @@
                                ) . '@s'
                        ),
                        array(
-                               new TimeValue( '+2001-01-01T00:00:00Z', 60, 0, 
1, 10, 'Stardate' ),
-                               $options,
+                               new TimeValue( '+2001-01-01T00:00:00Z', 0, 0, 
0, $day, 'Stardate' ),
                                '@.*<td 
class="wb-time-calendar">Stardate</td>.*@s'
                        ),
                        array(
-                               new TimeValue( '+2001-01-01T00:00:00Z', -179, 
0, 1, 10, TimeFormatter::CALENDAR_GREGORIAN ),
-                               $options,
+                               new TimeValue( '+2001-01-01T00:00:00Z', -179, 
0, 0, $day, $gregorian ),
                                '@.*<td 
class="wb-time-timezone">\xE2\x88\x9202:59</td>.*@s'
                        ),
                );

-- 
To view, visit https://gerrit.wikimedia.org/r/188541
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Iab1782a7fe1193cef2c186a9d4a2a2d6a4fa3a6d
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to