Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

merged branch mtdowling/ee0cdaa4ad6e3cbab25227785f49a75b9fd828f4 (PR #18

)

Commits
-------

a78e1f7 Using array_key_exists for raw instead of isset. Adding coverage.

Discussion
----------

Adding test coverage for raw

---------------------------------------------------------------------------

by igorw at 2011/08/16 07:25:29 -0700

Thanks, one minor thing. The naming is inconsistent with `testOffsetGetValidatesKeyIsPresent`. Key vs Identifier.

---------------------------------------------------------------------------

by mtdowling at 2011/08/17 12:16:16 -0700

I corrected the naming inconsistency and modified the getRaw() method to use array_key_exists instead of isset.

---------------------------------------------------------------------------

by igorw at 2011/08/17 13:56:53 -0700

Please, `testRawWithNullValue()` instead of testing two things in one test.

---------------------------------------------------------------------------

by mtdowling at 2011/08/17 14:08:25 -0700

Done.  Used the same naming convention as testOffsetGetHonorsNullValues().

---------------------------------------------------------------------------

by igorw at 2011/08/17 14:56:57 -0700

Looks good to me.

---------------------------------------------------------------------------

by fabpot at 2011/08/18 10:15:48 -0700

Can you squash your 3 commits? Thanks.

On a side note, what about using `array_key_exists()` in `offsetExists()` too?

---------------------------------------------------------------------------

by mtdowling at 2011/08/18 11:05:54 -0700

Sure thing. I'll try to figure it out and update the pull request.

I'm not sure about changing `offsetExists()`... Calling `isset()` on a null value returns false.  Because `offsetExists()` is triggered by an `isset()` call, I think it should return false for keys that are set to null values so that it is consistent with standard array behavior.

```php
<?php
$a = array("a" => null);
var_export(isset($a["a"]));
```
outputs `false`

---------------------------------------------------------------------------

by igorw at 2011/11/08 04:55:26 -0800

I would keep the isset() in offsetExists. If you need help with the squashing, send me a private message here on github.

---------------------------------------------------------------------------

by mtdowling at 2011/11/08 19:23:52 -0800

Sorry that took so long.  I squashed it into one commit.  Let me know if that doesn't look right.
  • Loading branch information...
commit 8c77a39cee7b995dd69a80d070d031c3ffcb6379 2 parents 22e0244 + a78e1f7
@fabpot authored
Showing with 18 additions and 1 deletion.
  1. +1 −1  lib/Pimple.php
  2. +17 −0 tests/Pimple/Tests/PimpleTest.php
View
2  lib/Pimple.php
@@ -139,7 +139,7 @@ function protect(Closure $callable)
*/
function raw($id)
{
- if (!isset($this->values[$id])) {
+ if (!array_key_exists($id, $this->values)) {
throw new InvalidArgumentException(sprintf('Identifier "%s" is not defined.', $id));
}
View
17 tests/Pimple/Tests/PimpleTest.php
@@ -165,4 +165,21 @@ public function testRaw()
$pimple['service'] = $definition = function () { return 'foo'; };
$this->assertSame($definition, $pimple->raw('service'));
}
+
+ public function testRawHonorsNullValues()
+ {
+ $pimple = new Pimple();
+ $pimple['foo'] = null;
+ $this->assertNull($pimple->raw('foo'));
+ }
+
+ /**
+ * @expectedException InvalidArgumentException
+ * @expectedExceptionMessage Identifier "foo" is not defined.
+ */
+ public function testRawValidatesKeyIsPresent()
+ {
+ $pimple = new Pimple();
+ $pimple->raw('foo');
+ }
}
Please sign in to comment.
Something went wrong with that request. Please try again.