[Serializer] error handling inconsistencies fixed in the serializer decoders #9586
Conversation
rodrigodiez
commented
Nov 23, 2013
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | yes |
| Tests pass? | yes |
| Fixed tickets | #9393 |
| License | MIT |
| Doc PR | none |
| if (!function_exists('json_last_error_msg')) { | ||
| switch (json_last_error()) { | ||
| default: | ||
| $message = 'Unknown error'; |
There was a problem hiding this comment.
Instead of using variable, you can use return:
if (!function_exists('json_last_error_msg')) {
switch (json_last_error()) {
default:
return 'Unknown error';
}
}
return json_last_error_msg();There was a problem hiding this comment.
Also, default should probably be the last case.
| switch (json_last_error()) { | ||
| default: | ||
| return 'Unknown error'; | ||
| break; |
There was a problem hiding this comment.
As you call return, the break is redundant here.
| @@ -26,6 +26,8 @@ | |||
| * @param array $context options that normalizers/encoders have access to. | |||
| * | |||
| * @return scalar | |||
| * | |||
| * @throws UnexpectedValueException | |||
There was a problem hiding this comment.
don't you need also here the use statement?
There was a problem hiding this comment.
Yep. Sorry guys, first code PR, still learning
| @@ -5,6 +5,8 @@ CHANGELOG | |||
| ----- | |||
|
|
|||
| * added `$context` support for XMLEncoder. | |||
| * [DEPRECATION] JsonEncode and JsonDecoder where modified to throw | |||
| * | ||
| * @see http://php.net/manual/en/function.json-last-error.php json_last_error | ||
| */ | ||
| * @deprecated Decoder throws exception if an error is found |
There was a problem hiding this comment.
The deprecation message should not be the short description of the method. It should go with the @see tag (the first line of the phpdoc is special).
and you should remove the extra indentation space you added.
|
Seems ok to you guys? |
|
Ping |
|
Closing this one in favor of #9876 as you seem to have removed your Symfony fork. |
…e serializer decoders (fabpot) This PR was merged into the 2.5-dev branch. Discussion ---------- [Serializer] error handling inconsistencies fixed in the serializer decoders | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #9393 | License | MIT | Doc PR | none see #9586 for the original PR Commits ------- a1ab939 [Serializer] fixed CS 6d9f0be Json encoder classes now throws UnexpectedValueException as XML classes