Jeroen De Dauw has uploaded a new change for review.
https://gerrit.wikimedia.org/r/77255
Change subject: Update ListDiffer and CallbackListDiffer to make use of new
ArrayComparer classes
......................................................................
Update ListDiffer and CallbackListDiffer to make use of new ArrayComparer
classes
Change-Id: I4437d21a5557d30a1031332f2653d3659c91b8ba
---
M src/ArrayComparer/StrategicArrayComparer.php
M src/differ/CallbackListDiffer.php
M src/differ/ListDiffer.php
M tests/phpunit/ArrayComparer/StrategicArrayComparerTest.php
M tests/phpunit/differ/CallbackListDifferTest.php
5 files changed, 67 insertions(+), 107 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Diff
refs/changes/55/77255/1
diff --git a/src/ArrayComparer/StrategicArrayComparer.php
b/src/ArrayComparer/StrategicArrayComparer.php
index fbf0977..555925a 100644
--- a/src/ArrayComparer/StrategicArrayComparer.php
+++ b/src/ArrayComparer/StrategicArrayComparer.php
@@ -3,6 +3,7 @@
namespace Diff\ArrayComparer;
use Diff\Comparer\ValueComparer;
+use RuntimeException;
/**
* Computes the difference between two arrays by comparing elements with
@@ -60,11 +61,16 @@
* @param array $haystack
*
* @return bool|int|string
+ * @throws RuntimeException
*/
protected function arraySearch( $needle, array $haystack ) {
foreach ( $haystack as $valueOffset => $thing ) {
$areEqual = $this->valueComparer->valuesAreEqual(
$needle, $thing );
+ if ( !is_bool( $areEqual ) ) {
+ throw new RuntimeException( 'ValueComparer
returned a non-boolean value' );
+ }
+
if ( $areEqual ) {
return $valueOffset;
}
diff --git a/src/differ/CallbackListDiffer.php
b/src/differ/CallbackListDiffer.php
index e8f9e11..ce8c21f 100644
--- a/src/differ/CallbackListDiffer.php
+++ b/src/differ/CallbackListDiffer.php
@@ -2,7 +2,8 @@
namespace Diff;
-use Exception;
+use Diff\ArrayComparer\StrategicArrayComparer;
+use Diff\Comparer\CallbackComparer;
/**
* Differ that only looks at the values of the arrays (and thus ignores key
differences).
@@ -21,11 +22,11 @@
class CallbackListDiffer implements Differ {
/**
- * @since 0.5
+ * @since 0.8
*
- * @var callable|null
+ * @var ListDiffer
*/
- protected $comparisonCallback = null;
+ protected $differ = null;
/**
* Constructor.
@@ -35,7 +36,7 @@
* @param callable $comparisonCallback
*/
public function __construct( $comparisonCallback ) {
- $this->comparisonCallback = $comparisonCallback;
+ $this->differ = new ListDiffer( new StrategicArrayComparer( new
CallbackComparer( $comparisonCallback ) ) );
}
/**
@@ -49,68 +50,7 @@
* @return DiffOp[]
*/
public function doDiff( array $oldValues, array $newValues ) {
- $operations = array();
-
- foreach ( $this->diffArrays( $newValues, $oldValues ) as
$addition ) {
- $operations[] = new DiffOpAdd( $addition );
- }
-
- foreach ( $this->diffArrays( $oldValues, $newValues ) as
$removal ) {
- $operations[] = new DiffOpRemove( $removal );
- }
-
- return $operations;
- }
-
- /**
- * @since 0.5
- *
- * @param array $arrayOne
- * @param array $arrayTwo
- *
- * @return array
- */
- protected function diffArrays( array $arrayOne, array $arrayTwo ) {
- $notInTwo = array();
-
- foreach ( $arrayOne as $element ) {
- $valueOffset = $this->arraySearch( $element, $arrayTwo
);
-
- if ( $valueOffset === false ) {
- $notInTwo[] = $element;
- continue;
- }
-
- unset( $arrayTwo[$valueOffset] );
- }
-
- return $notInTwo;
- }
-
- /**
- * @since 0.5
- *
- * @param string|int $needle
- * @param array $haystack
- *
- * @return bool|int|string
- * @throws Exception
- */
- protected function arraySearch( $needle, array $haystack ) {
- foreach ( $haystack as $valueOffset => $thing ) {
- $areEqual = call_user_func_array(
$this->comparisonCallback, array( $needle, $thing ) );
-
- if ( !is_bool( $areEqual ) ) {
- // TODO: throw a more specific exception type
- throw new Exception( 'Comparison callback
returned a non-boolean value' );
- }
-
- if ( $areEqual ) {
- return $valueOffset;
- }
- }
-
- return false;
+ return $this->differ->doDiff( $oldValues, $newValues );
}
}
diff --git a/src/differ/ListDiffer.php b/src/differ/ListDiffer.php
index 757d130..108795c 100644
--- a/src/differ/ListDiffer.php
+++ b/src/differ/ListDiffer.php
@@ -2,13 +2,16 @@
namespace Diff;
+use Diff\ArrayComparer\ArrayComparer;
+use Diff\ArrayComparer\NativeArrayComparer;
use Diff\ArrayComparer\StrictArrayComparer;
+use InvalidArgumentException;
/**
* Differ that only looks at the values of the arrays (and thus ignores key
differences).
*
- * Values are compared using the strictDiff method in strict mode (default)
- * or using array_diff_assoc in native mode.
+ * By default values are compared using a StrictArrayComparer.
+ * You can alter the ArrayComparer used by providing one in the constructor.
*
* @since 0.4
*
@@ -25,6 +28,7 @@
* This makes use of @see array_diff
*
* @since 0.4
+ * @deprecated since 0.8
*/
const MODE_NATIVE = 0;
@@ -33,31 +37,48 @@
* This makes use of @see ListDiffer::strictDiff
*
* @since 0.4
+ * @deprecated since 0.8
*/
const MODE_STRICT = 1;
/**
- * @since 0.4
+ * @since 0.8
*
- * @var int
+ * @var ArrayComparer
*/
- protected $diffMode;
+ protected $arrayComparer;
/**
- * Constructor.
+ * @param ArrayComparer $arrayComparer
*
- * Takes an argument that determines the diff mode.
- * By default this is ListDiffer::MODE_STRICT, which causes
- * computation in @see doDiff to be done via @see arrayDiff.
- * If the native behavior is preferred, ListDiffer::MODE_NATIVE
- * can be specified.
- *
- * @since 0.4
- *
- * @param int $diffMode
+ * The first argument is an ArrayComparer since version 0.8.
+ * Before this it was an element of the ListDiffer::MODE_ enum.
+ * The later is still supported though deprecated.
*/
- public function __construct( $diffMode = self::MODE_STRICT ) {
- $this->diffMode = $diffMode;
+ public function __construct( $arrayComparer = null ) {
+ $this->arrayComparer = $this->getRealArrayComparer(
$arrayComparer );
+ }
+
+ /**
+ * @param $arrayComparer
+ *
+ * @return ArrayComparer
+ * @throws InvalidArgumentException
+ */
+ protected function getRealArrayComparer( $arrayComparer ) {
+ if ( $arrayComparer === null || $arrayComparer ===
self::MODE_STRICT ) {
+ return new StrictArrayComparer();
+ }
+
+ if ( $arrayComparer === self::MODE_NATIVE ) {
+ return new NativeArrayComparer();
+ }
+
+ if ( is_object( $arrayComparer ) && $arrayComparer instanceof
ArrayComparer ) {
+ return $arrayComparer;
+ }
+
+ throw new InvalidArgumentException( 'Got an invalid
$arrayComparer' );
}
/**
@@ -94,28 +115,7 @@
* @return array
*/
protected function diffArrays( array $arrayOne, array $arrayTwo ) {
- if ( $this->diffMode === self::MODE_STRICT ) {
- return $this->strictDiff( $arrayOne, $arrayTwo );
- }
- else {
- return array_diff( $arrayOne, $arrayTwo );
- }
- }
-
- /**
- * Returns an array containing all the entries from arrayOne that are
not present in arrayTwo.
- *
- * @since 0.4
- *
- * @param array $arrayOne
- * @param array $arrayTwo
- *
- * @return array
- */
- protected function strictDiff( array $arrayOne, array $arrayTwo ) {
- $differ = new StrictArrayComparer();
-
- return $differ->diffArrays( $arrayOne, $arrayTwo );
+ return $this->arrayComparer->diffArrays( $arrayOne, $arrayTwo );
}
}
diff --git a/tests/phpunit/ArrayComparer/StrategicArrayComparerTest.php
b/tests/phpunit/ArrayComparer/StrategicArrayComparerTest.php
index b457c0f..80e5cd4 100644
--- a/tests/phpunit/ArrayComparer/StrategicArrayComparerTest.php
+++ b/tests/phpunit/ArrayComparer/StrategicArrayComparerTest.php
@@ -201,4 +201,18 @@
);
}
+ public function testCallbackComparisonReturningNyanCat() {
+ $valueComparer = $this->getMock( 'Diff\Comparer\ValueComparer'
);
+
+ $valueComparer->expects( $this->once() )
+ ->method( 'valuesAreEqual' )
+ ->will( $this->returnValue( '~=[,,_,,]:3' ) );
+
+ $arrayComparer = new StrategicArrayComparer( $valueComparer );
+
+ $this->setExpectedException( 'RuntimeException' );
+
+ $arrayComparer->diffArrays( array( 1, '2', 'baz' ), array( 1,
'foo', '2' ) );
+ }
+
}
diff --git a/tests/phpunit/differ/CallbackListDifferTest.php
b/tests/phpunit/differ/CallbackListDifferTest.php
index ad0e5e9..d8f8bdc 100644
--- a/tests/phpunit/differ/CallbackListDifferTest.php
+++ b/tests/phpunit/differ/CallbackListDifferTest.php
@@ -221,7 +221,7 @@
return '~=[,,_,,]:3';
} );
- $this->setExpectedException( 'Exception' );
+ $this->setExpectedException( 'RuntimeException' );
$differ->doDiff( array( 1, '2', 'baz' ), array( 1, 'foo', '2' )
);
}
--
To view, visit https://gerrit.wikimedia.org/r/77255
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4437d21a5557d30a1031332f2653d3659c91b8ba
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Diff
Gerrit-Branch: master
Gerrit-Owner: Jeroen De Dauw <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
