json issue and the array-set helper #33

Closed
wants to merge 4 commits into
from

Projects

None yet

2 participants

@kkamkou
kkamkou commented Mar 14, 2012

Hello Jonathan,
Another small part of code. Please, review.

@a-musing-moose
Owner

Hi Kkamkou,

The setFromArray() function I think is unneeded. It does pretty much the same thing as __setData() or am I missing something?

@kkamkou
kkamkou commented Apr 4, 2012

Hello Jonathan,
__setData creates new Generic property if it doesn't exist. setFromArray is just useful helper (for me, but you should decide).

@kkamkou kkamkou commented on an outdated diff Apr 6, 2012
src/morph/Iterator.php
$object = new $class;
- $object->__setData($item, Enum::STATE_CLEAN)
- ->collection($this->type->collection());
@kkamkou
kkamkou Apr 6, 2012

it should be here.

@a-musing-moose
Owner

kkamkou - It seems you have committed other code to your repo since opening this pull request. As a result this pull request also includes those commits which shouldn't be merged in.

Also could we not add an optional flag '$addNewProperties = true' to __setData rather than add a new function? If the default is true it would be backward compatible and in your case you just set it to false?

So if you would like this feature please can you add it as an optional feature of __setData in a fresh branch by itself and open another pull request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment