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

added functions to iterate on objects #7

Merged
merged 6 commits into from
Jun 13, 2015

Conversation

nikhedonia
Copy link
Contributor

In my opinion, a must have for trine.

This enables us to work with objects in an idiomatic way;
Let's get rid of this cumbersome Object.keys(obj) and replace it with:

var obj={a:1,b:2,c:3};
obj::keys()  // yields  keys of an object ( here : 'a','b','c' )
obj::values() // yields values of an object  ( here : 1,2,'3 )

// and to reduce repetition:

obj::pairs() // yield {key:'a',value:1} ... you get the idea 

I think a excellent source of inspiration is python's itertools...
In my next commit i'd like to introduce enumerate, zip, and some other functions

@jussi-kalliokoski
Copy link
Owner

Thanks for taking the time to contribute! ❤️

First things first: this should go into a category of its own: object. The category is meant to indicate what the type of this should be.

Other than that, instead of pairs, it should be entries and use the Object.entries function. That's consistent with how other key-value pairs are represented as iterators, e.g. [1,2,3].entries(), new Map(["foo", "bar"]).entries(). values can also use Object.values().

So ideally, these would just be wrappers around Object.keys(), Object.values() and Object.entries() that take the object as this and return an iterator instead of an array.

In my next commit i'd like to introduce enumerate, zip, and some other functions

Sounds great! Please make them separate PRs though. ;)

…are now wrappers to Object.values() and Object.entries()
* ```
*/
export function * keys () : Iterable<String> {
yield* Object.keys(this);

Choose a reason for hiding this comment

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

Elsewhere in codebase the spacing with yield* is yield *

Choose a reason for hiding this comment

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

also, four space indentation

@jussi-kalliokoski
Copy link
Owner

Sorry to nitpick about code style, haven't had the time to setup JSCS for the project yet, and unfortunately JSCS doesn't even have rules for a lot of things in the codebase yet. :) Looks good otherwise!

* @example Basic Usage
*
* ```javascript
* {a:1,b:2,c:3}::pairs()

Choose a reason for hiding this comment

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

This should be entries.

@nikhedonia
Copy link
Contributor Author

ahhh, i feel your pain ;P
I see, i pushed too fast; ill be more careful!

@jussi-kalliokoski
Copy link
Owner

Sorry, and thanks for your patience! :D ❤️ Actually just re-checked the Flow docs and it should be Object as you originally had, and Flow has support for tuples, so we should use that for the entries - I'll put comments inline

/**
* Yields entries of the bound object.
*
* @this {Object<T>}

Choose a reason for hiding this comment

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

Back to {Object} :D

@nikhedonia
Copy link
Contributor Author

haha... i'm today just to fast at pushing ... i drank to much coffee, i guess...

@jussi-kalliokoski
Copy link
Owner

haha... i'm today just to fast at pushing ... i drank to much coffee, i guess...

happens to the best of us :D

@jussi-kalliokoski
Copy link
Owner

Awesome, thank you! ❤️ 👍 🎉

jussi-kalliokoski added a commit that referenced this pull request Jun 13, 2015
added functions to iterate on objects
@jussi-kalliokoski jussi-kalliokoski merged commit 1c7efc4 into jussi-kalliokoski:master Jun 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants