Skip to content

Commit

Permalink
Use array_key_exists instead of isset()
Browse files Browse the repository at this point in the history
This allows contains() to work with null values.

Fixes #4083
  • Loading branch information
markstory committed Sep 20, 2013
1 parent 68aefe7 commit a30f861
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 2 deletions.
6 changes: 6 additions & 0 deletions lib/Cake/Test/Case/Utility/HashTest.php
Expand Up @@ -593,6 +593,12 @@ public function testContains() {
);
$this->assertTrue(Hash::contains($b, $a));
$this->assertFalse(Hash::contains($a, $b));

$a = array(0 => 'test', 'string' => null);
$this->assertTrue(Hash::contains($a, array('string' => null)));

$a = array(0 => 'test', 'string' => null);
$this->assertTrue(Hash::contains($a, array('test')));
}

/**
Expand Down
4 changes: 2 additions & 2 deletions lib/Cake/Utility/Hash.php
Expand Up @@ -442,14 +442,14 @@ public static function contains(array $data, array $needle) {
$val = $needle[$key];
unset($needle[$key]);

if (isset($data[$key]) && is_array($val)) {
if (array_key_exists($key, $data) && is_array($val)) {
$next = $data[$key];
unset($data[$key]);

if (!empty($val)) {
$stack[] = array($val, $next);
}
} elseif (!isset($data[$key]) || $data[$key] != $val) {
} elseif (!array_key_exists($key, $data) || $data[$key] != $val) {
return false;
}

Expand Down

7 comments on commit a30f861

@EliuFlorez
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory redimiento improving the result of php code faster with less time evaluating the array is like this: if (isset ($ data [$ key]))

@Phally
Copy link
Contributor

@Phally Phally commented on a30f861 Sep 20, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you read the commit message and the ticket?

@EliuFlorez
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No.

@dereuromark
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should :)

@josegonzalez
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol.

@markstory
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ElluFlorez I'm well aware of the performance differences between isset() and array_key_exists(). This change intentionally switched to array_key_exists() to solve specific bugs.

@EliuFlorez
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok I understand the reason. : D @ markstory regards

Please sign in to comment.