Initial implementation #1
Conversation
|
When merging please do |
composer.json
Outdated
| @@ -0,0 +1,28 @@ | |||
| { | |||
| "name": "lendable/json", | |||
| "description": "Json Abstraction", | |||
There was a problem hiding this comment.
| "description": "Json Abstraction", | |
| "description": "JSON serializer/deserializer with an OOP interface", |
lib/JsonDeserializeFailed.php
Outdated
|
|
||
| namespace Lendable\Json; | ||
|
|
||
| final class JsonDeserializeFailed extends \RuntimeException implements JsonFailure |
There was a problem hiding this comment.
nit: JsonDeserializationFailed ?
There was a problem hiding this comment.
I am thinking, do we even need the Json prefix at all?
There was a problem hiding this comment.
yeah fair enough, namespace exists
lib/JsonSerializeFailed.php
Outdated
|
|
||
| namespace Lendable\Json; | ||
|
|
||
| final class JsonSerializeFailed extends \RuntimeException implements JsonFailure |
There was a problem hiding this comment.
JsonSerializationFailed ?
lib/JsonSerializer.php
Outdated
| return $serialized; | ||
| } | ||
|
|
||
| public function deserialize(string $json): array |
There was a problem hiding this comment.
This isn't a safe return type, to make it safe we'd need to A) document that only objects/arrays are supported (i'm fine with that) and B) check the result after json_decode() and throw another exception (InvalidDeserializedData?) if not an array.
There was a problem hiding this comment.
fixed, along with other stuff
lib/Serializer.php
Outdated
|
|
||
| namespace Lendable\Json; | ||
|
|
||
| interface Serializer |
There was a problem hiding this comment.
Don't think we need this unless you can convince me of it's value :)
|
LMK thoughts on above suggestions, doesn't have to be done |
Co-Authored-By: kunicmarko20 <kunicmarko20@gmail.com>
lib/Serializer.php
Outdated
| final class Serializer | ||
| { | ||
| /** | ||
| * @param mixed $data |
|
So, can we merge this? |
No description provided.