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

Implementing methods suggested on issue #103. #104

Closed
wants to merge 2 commits into from
Closed

Implementing methods suggested on issue #103. #104

wants to merge 2 commits into from

Conversation

rafaelrinaldi
Copy link

List of modules implemented:

  • object/camelCaseKeys
  • object/hyphenateKeys
  • object/mapKeys
  • object/underscoreKeys

Tests are passing and the documentation was also updated.

closes #103

lorem : 3
};
values(obj); // [1, 2, 3]
```
Copy link
Member

Choose a reason for hiding this comment

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

missing a line break at the end of the file (might cause errors on some systems)

@millermedeiros
Copy link
Member

also need to make sure we test the behavior for nested properties like:

{
  "lorem_ipsum" : {
    "dolor_sit" : "amet"
   }
}

see comment for more details on proposed feature/implementation

@rafaelrinaldi
Copy link
Author

I would like to ask you guys what's best here... Should I create individual modules for "deep" mapping keys (deepCamelCaseKeys(obj)) or it should be a parameter (camelCaseKeys(obj, true))?

@millermedeiros @conradz

@roboshoes
Copy link
Member

Not really up to me. But I would say, before you make it a Boolean why don't you make it an integer that defines the levels of how deep you go. So only the first level would be camelCaseKeys( obj, 1 ) and leaving it out camelCaseKeys( obj ) is the equivalent to deep mapping.

However, I'm fine with two packages.

@millermedeiros
Copy link
Member

I would also implement with a depth argument instead of a boolean, that way we avoid the boolean trap. By default it would be deep (recursive), just like we do with the array/flatten method

@millermedeiros
Copy link
Member

I moved this to the v0.8 milestone (that should be released next month) since @rafaelrinaldi and myself are busy and v0.7 is schedule for this week (and we already delayed it for too long).

@conradz
Copy link
Member

conradz commented Feb 3, 2014

Visiting this again, it seems that mapKeys should be pretty much all we need (and possibly a deepMapKeys if desired). Instead of camelCaseKeys, you could simply do mapKeys(obj, camelCase), which is only a few characters longer. The camelCaseKeys, etc. modules could all simply be done in a one-line function (e.g. return mapKeys(obj, camelCase);), which I think is simply an added layer of indirection without many benefits.

Any thoughts?

@millermedeiros
Copy link
Member

@conradz I agree. It's not hard to implement and can be useful in many cases. Just not sure if we need 2 methods or if we should accept depth as 3rd argument (and do a deep map by default).

@rafaelrinaldi
Copy link
Author

@conradz @millermedeiros Should I then ditch everything but mapKeys? Also, deepMapKeys or depth as mapKeys parameter?

@conradz
Copy link
Member

conradz commented Feb 4, 2014

I would be 👍 on only having mapKeys and a deepMapKeys, to maintain consistency with other object methods.

@millermedeiros
Copy link
Member

@conradz yeah, I agree; we use 2 methods everywhere, let's keep it consistent. - depth can be an optional argument if that makes implementation simpler (like string/crop is just a sugared version of string/truncate)

@millermedeiros
Copy link
Member

closing this since we probably just want a mapKeys and deepMapKeys.

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.

object/mapKeys, object/camelCaseKeys, object/hyphenateKeys and object/underscoreKeys
4 participants