Closed phindmarsh closed 10 years ago
Hi
First: If you try to fix different things do it in different PRs. Second: String quotes was ok. The intent is tell you the param you must fix and not the value. Third: For the rest of fixes please add unit testing.
Thanks
Ok, I've removed the other changes.
Now it's just a change from in_array
to array_key_exists
plus the associated tests (testDuplicateDataKeyThrowsException
).
String interpolation only works with double quoted strings and the exception messages have variables included, but are single quoted. This caused exception messages like
$key must be a non-empty string
.in_array
in theaddData
method should bearray_key_exists
, since the purpose of the method is to ensure existing$this->data
keys are not overwritten.in_array
checks if a value is present in an array, not a key.Also, using
in_array
without strict checking can cause some unpredictable results (see docs). In this case, I think it is better to use the strict flag in theaddRegistrationId
method.