Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

[2.3] [Serializer] Enabled camelCase format to be used on denormalize method #6951

Merged
merged 1 commit into from Mar 23, 2013

Conversation

Projects
None yet
4 participants
Contributor

marcosQuesada commented Feb 2, 2013

Enabled camelCase formater , that way when hydrating from arrays, attributes as attribute_name could be implemented as attributteName parameter, with getAttributeName and setAttributeName, giving different formating option from setAttribute_name getAttribute_name.

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets no
License MIT
Doc PR no
Owner

fabpot commented Feb 4, 2013

@lsmith77 lsmith77 commented on an outdated diff Feb 5, 2013

...nent/Serializer/Normalizer/GetSetMethodNormalizer.php
@@ -142,6 +143,22 @@ public function denormalize($data, $class, $format = null, array $context = arra
}
/**
+ * Format attribute name to access parameters or methods
+ *
+ * @param string $attribute
+ * @param string $format by default is null
+ * @return string
+ */
+ protected function formatAttribute($attribute, $format)
+ {
+ if ($format == 'camelCase') {
@lsmith77

lsmith77 Feb 5, 2013

Contributor

this is sort of abusing the $format which should be a format like xml or json ..
maybe a switch like we have for ignoredAttributes would make more sense?

Contributor

marcosQuesada commented Feb 5, 2013

I understand the mission of format on normalize method, but in denormalize case I don't see it as crearly, cause as a result we obtain an hydrated object, so i understand that has nothing to do with Json or Xml.
Anyway, I understand that normalize & denormalize uses the same base concepts.... so, when you say to use something like ignoredAttributes, did you mean to use a setter on camelizedAttributes?, and that way, using it on formatAttribute removing dependence from format, isn't it?

Contributor

lsmith77 commented Feb 5, 2013

yes.

Contributor

marcosQuesada commented Feb 5, 2013

Ok I agree with that! , I expect to have this PR updated as you say as soon as posible, possibly, tonight!

Contributor

marcosQuesada commented Feb 5, 2013

Done!, now it's using camelizedAttributes, that if needed, has to be set before invoking denormalize , I think that is a cleanner implementation.

Contributor

ricardclau commented Feb 5, 2013

I agree with @marcosQuesada when he says that it seems there is no point in having a format parameter when denormalizing. What is the point of having it? I don't see a clear use case

@lsmith77 lsmith77 commented on an outdated diff Feb 5, 2013

...nent/Serializer/Normalizer/GetSetMethodNormalizer.php
@@ -142,6 +154,21 @@ public function denormalize($data, $class, $format = null, array $context = arra
}
/**
+ * Format attribute name to access parameters or methods
+ *
+ * @param string $attribute
+ * @return string
+ */
+ protected function formatAttribute($attributeName)
+ {
+ if (in_array($attributeName, $this->camelizedAttributes)) {
+ return preg_replace_callback('/(^|_|\.)+(.)/', function ($match) { return ('.' === $match[1] ? '_' : '').strtoupper($match[2]); }, $attributeName);
@lsmith77

lsmith77 Feb 5, 2013

Contributor

can you add some new lines to the definition of the closure?

@lsmith77

lsmith77 Feb 6, 2013

Contributor

the reason being better readability

@lsmith77 lsmith77 commented on an outdated diff Feb 5, 2013

...lizer/Tests/Normalizer/GetSetMethodNormalizerTest.php
$this->normalizer->normalize($obj, 'any')
);
}
+
@lsmith77

lsmith77 Feb 5, 2013

Contributor

please remove the extra new line

@lsmith77 lsmith77 commented on the diff Feb 5, 2013

...lizer/Tests/Normalizer/GetSetMethodNormalizerTest.php
@@ -43,6 +45,40 @@ public function testDenormalize()
$this->assertEquals('bar', $obj->getBar());
}
+
@lsmith77

lsmith77 Feb 5, 2013

Contributor

please remove the extra new line

Contributor

lsmith77 commented Feb 5, 2013

@ricardclau: there might be some case where the decoder cannot do a perfect job. f.e. i could imagine some edge case situations with dashes and underscores. but yeah i am not aware of any core denormalizer using this parameter.

Contributor

marcosQuesada commented Feb 5, 2013

Done! Interesting case @lsmith77.

@marcosQuesada marcosQuesada Enabled camelCase format to be used on denormalize method, that way c…
…amel_case attribute can be used on object as getCamelCase()
fbffdf0
Contributor

marcosQuesada commented Feb 6, 2013

Updated readability on closure, did you mean that? let me now if there's anything more to correct

Contributor

lsmith77 commented Feb 6, 2013

thx .. looks good to me now @fabpot

Contributor

marcosQuesada commented Feb 13, 2013

@fabpot Is there anything else to add? Maybe, I have to prepare documentation about that ..? thx @lsmith77

Contributor

lsmith77 commented Feb 13, 2013

I guess this should go into 2.3

Owner

fabpot commented Feb 13, 2013

Indeed, this will be considered for 2.3. In the meantime, you should send a pull request to the documentation repository.

Contributor

marcosQuesada commented Feb 13, 2013

ok, i will work on it! thx

@fabpot fabpot added a commit that referenced this pull request Mar 23, 2013

@fabpot fabpot merged branch marcosQuesada/serializer/denormalize-camelcase (PR #6951)
This PR was merged into the master branch.

Discussion
----------

[2.3] [Serializer] Enabled camelCase format to be used on denormalize method

 Enabled camelCase formater , that way when hydrating from arrays, attributes as attribute_name could be implemented as attributteName parameter, with getAttributeName and setAttributeName, giving different formating option from setAttribute_name  getAttribute_name.

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | no
| License       | MIT
| Doc PR        | no

Commits
-------

fbffdf0 Enabled camelCase format to be used on denormalize method, that way camel_case attribute can be used on object as getCamelCase()
f465d2a

@fabpot fabpot closed this Mar 23, 2013

@fabpot fabpot merged commit fbffdf0 into symfony:master Mar 23, 2013

1 check failed

default The Travis build could not complete due to an error
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment