thirtybees / blocklayered

Academic Free License v3.0
0 stars 5 forks source link

You were Drunk or so what ? #7

Closed Captain-FLAM closed 5 years ago

Captain-FLAM commented 5 years ago

Read my code...

Traumflug commented 5 years ago

If you'd care to explain us what this commit changes, one might see the sense of it. Something like

$data = json_decode($data, true);

should be entirely fine.

yaniv14 commented 5 years ago

@Traumflug I think he got a good point. this $data = json_decode($data, true); will cause $data value to get lost in this:

if ( ! $data) {
    $data = Tools::unSerialize($data);
}
Traumflug commented 5 years ago

this $data = json_decode($data, true); will cause $data value to get lost in this:

if ( ! $data) {
    $data = Tools::unSerialize($data);
}

Yes, and that's actually wanted. Up to date data is encoded as JSON, data stored before the move to JSON is stored with PHP serialize(). Accordingly, try to decode as JSON first, and if this fails (result is empty), decode with unserialize(). A strategy called "retrocompatibility" (I like this word) in PS parlance.

yaniv14 commented 5 years ago

@Traumflug I understand your intention for retrocompatibility, but you are missing the point. when this $data = json_decode($data, true); returns null/empty than the original $data value is gone. how can you unSerialize null? you will want to try and unSerialize the original $data value.

Traumflug commented 5 years ago

Now I get it. Thanks.

Captain-FLAM commented 5 years ago

Well done @yaniv14 :1st_place_medal:

I was too busy to explain it ... Sorry

So now, you can merge it !

P.S : Next time, please think that if I pull a request, it's not accidentally ;-)

Captain-FLAM commented 5 years ago

Not yet integrated ???

Traumflug commented 5 years ago

Alright, picked to master as 69933a5206601ec144e17749cd9da4ba55c82f0c. Thank you very much!