[WIP] Update library/Zend/Stdlib/Hydrator/ClassMethods.php #3754

Closed
wants to merge 10 commits into
from

Conversation

Projects
None yet
7 participants
Contributor

mabuzagu commented Feb 12, 2013

ucfirst at the previous point forces you to have lowercase first properties
whereas if you are dealing with an existing table that already has
Uppercase first, then the Hydrator wont work. The fix above maintains everything
you had before but also allows for the use of uppercase first property e.g.
protected $FirstName;  MSSQL tables usually have this naming convention

@mabuzagu mabuzagu Update library/Zend/Stdlib/Hydrator/ClassMethods.php
ucfirst at the previous point forces you to have lowercase first properties
whereas if you are dealing with an existing table that already has
Uppercase first, then the Hydrator wont work. The fix above maintains everything
you had before but also allows for the use of uppercase first property e.g. 
protected $FirstName;  MSSQL tables usually have this naming convention
51a60c6

@samsonasik samsonasik commented on an outdated diff Feb 12, 2013

library/Zend/Stdlib/Hydrator/ClassMethods.php
}
if ($this->underscoreSeparatedKeys) {
$attribute = preg_replace_callback('/([A-Z])/', $transform, $attribute);
}
- $attributes[$attribute] = $this->extractValue($attribute, $object->$method());
+
+
+ if(property_exists($object, $attribute)) {
+ $attributes[$attribute] = $this->extractValue($attribute, $object->$method());
+ } else {
+ $attribute = lcfirst($attribute);
+ $attributes[$attribute] = $this->extractValue($attribute, $object->$method());
+ }
Contributor

samsonasik commented Feb 12, 2013

I think this make tests failure in tests\ZendTest\Stdlib\HydratorStrategyTest and tests\ZendTest\Stdlib\HydratorTest

Member

ralphschindler commented Feb 15, 2013

Hi @mabuzagu
Can you please have a look at the above commentary and address their concerns?
Thanks!

EvanDotPro was assigned Feb 15, 2013

@mabuzagu mabuzagu Update library/Zend/Stdlib/Hydrator/ClassMethods.php
Formated according to formatting standars
51d0989
Contributor

mabuzagu commented Feb 18, 2013

Thanks for the comments samsonasik and ralph. I have made the changes and it looks like the tests are running ok

Owner

weierophinney commented Feb 19, 2013

@mabuzagu Please add a unit test for the new behavior, to ensure we don't have a future regression that removes this capability.

Thanks!

@Freeaqingme Freeaqingme and 1 other commented on an outdated diff Mar 6, 2013

library/Zend/Stdlib/Hydrator/ClassMethods.php
}
if ($this->underscoreSeparatedKeys) {
$attribute = preg_replace_callback('/([A-Z])/', $transform, $attribute);
}
- $attributes[$attribute] = $this->extractValue($attribute, $object->$method());
+
+
+ if(property_exists($object, $attribute)) {
+ $attributes[$attribute] = $this->extractValue($attribute, $object->$method());
+ } else {
@Freeaqingme

Freeaqingme Mar 6, 2013

Member

Could you perhaps correct the indenting here (and the line above this, and the block below as well)? If it all looks okay in your editor, please make sure you use spaces. Not tabs. (we try to adhere to PSR2, which uses 4 spaces per tab: https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-2-coding-style-guide.md )

Thanks!

@mabuzagu

mabuzagu Mar 11, 2013

Contributor

Thanks. Will do

Owner

weierophinney commented Mar 11, 2013

@mabuzagu Any progress on the unit tests? If you need help, jump into #zftalk.dev on Freenode IRC, and ask for assistance -- lots of folks who will be able to help you there!

Contributor

mabuzagu commented Mar 11, 2013

Sorry about the delay. Pass couple week have been hell :). But I think this week I should be able to get to it. I started on a some time back but seemed my solution was still causing the UT to fail. So I need to look at it more closely. You will have something from me soon. Sorry for the delay

Owner

weierophinney commented Apr 11, 2013

@mabuzagu Will you be able to finish this? If not, I'll close, and you can re-submit when you have tests. At this point, your branch is getting more and more out-of-date, and I worry that the merge will be difficult.

Contributor

mabuzagu commented Apr 12, 2013

I gave it a try and of course the unit tests failed :) Give me until the Monday. I should be able to lock myself somewhere and do it. I just need to different approach and some thorough testing. Thanks for your patience

Contributor

mabuzagu commented Apr 14, 2013

@weierophinney. Its ready now. Tests are good to go. Thanks for your patience

iquabius referenced this pull request Apr 15, 2013

Closed

CS fixes for #3754 #4229

Contributor

iquabius commented Apr 15, 2013

CS fixes for this are on #4229.

@weierophinney weierophinney added a commit that referenced this pull request Apr 15, 2013

@weierophinney weierophinney Merge branch 'feature/3754' into develop
Close #3754
Close #4229
e1091c3
Owner

weierophinney commented Apr 15, 2013

As this provides a new feature, I've merged it to the develop branch for release with 2.2.0. Thanks!

@weierophinney weierophinney added a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015

@weierophinney weierophinney Merge pull request zendframework/zendframework#3754 from mabuzagu/pat…
…ch-1

[WIP] Update library/Zend/Stdlib/Hydrator/ClassMethods.php
f0e1784
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment