Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add function for converting strings in snake_case to CamelCase #23

Merged
merged 4 commits into from Nov 22, 2016

Conversation

sctape
Copy link
Contributor

@sctape sctape commented Nov 22, 2016

No description provided.

Copy link

@bvisness bvisness left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good to me, but I'm curious - what is the use case for this, and is it a common enough one to include in this library?

'snake',
'snake',
false,
]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed you had some special handling for extra underscores - it might be good to see some tests of that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call.

@bvisness
Copy link

(@shadowhand presumably has a fix in the works for the Travis failures, by the way.)

*/
function snakeToCamelCase($snake_string, $first = false)
{
$camel = implode('', array_map(function($piece) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be a static function as a micro optimization.

@shadowhand
Copy link
Contributor

I remember seeing those errors with Travis before, but I am not sure what they are caused by.

@sctape
Copy link
Contributor Author

sctape commented Nov 22, 2016

@bvisness I need the function for use in __set() in an entity. I want to convert an entity's property from snake_case to CamelCase to look for a matching setter function in the entity.

$entity->first_name = 'John' would become $entity->setFirstName('John')

@bvisness
Copy link

I say go for it. (Once the Travis issues are resolved, anyway... @shadowhand, perhaps it is an old version of PHPUnit again?)

@shadowhand
Copy link
Contributor

@bvisness this build had no issues: https://travis-ci.org/equip/assist/jobs/155386468

function snakeToCamelCase($snake_string, $first = false)
{
$camel = implode('', array_map(static function($piece) {
return empty($piece) ? '' : ucfirst(strtolower($piece));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using array_filter would remove the need for this ternary.

{
$camel = implode('', array_map(static function($piece) {
return empty($piece) ? '' : ucfirst(strtolower($piece));
}, explode('_', $snake_string)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replacing this explode with preg_split('/_++/', $snake_string) would also remove the need for the ternary, as empty values would be impossible.

The most recent version of lstrojny/functional-php (1.4) is incompatible
with HHVM so we much lock the version's upper bound in order for equip/assist
to continue to work on HHVM.
@sctape sctape force-pushed the feature/camel-case-converter branch from f4a41a6 to b8651d8 Compare November 22, 2016 21:12
@sctape
Copy link
Contributor Author

sctape commented Nov 22, 2016

@shadowhand @bvisness Looks like lstrojny/functional-php:1.4 is incompatible with hhvm. I locked the upper bound of lstrojny/functional-php at 1.2.4 to resolve the issue. If in the future we decide to drop HHVM support, or the HHVM team resolves this issue, we can remove the explicit composer dependency.

@shadowhand
Copy link
Contributor

shadowhand commented Nov 22, 2016

👍

Approved with PullApprove

@sctape sctape merged commit d6eccde into master Nov 22, 2016
@sctape sctape deleted the feature/camel-case-converter branch November 22, 2016 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants