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 once to Utils #186

Merged
merged 1 commit into from
Oct 17, 2013
Merged

Add once to Utils #186

merged 1 commit into from
Oct 17, 2013

Conversation

psyduk
Copy link

@psyduk psyduk commented Oct 16, 2013

add once functionality to utils. Inspired by underscore/lodash.

if (ran) {
return result;
}
ran = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this appear after the function has been called?

Copy link
Author

Choose a reason for hiding this comment

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

will change.

Copy link
Contributor

Choose a reason for hiding this comment

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

still needs to be fixed

@necolas
Copy link
Contributor

necolas commented Oct 16, 2013

Also needs documentation added to the doc/utils_api.md file

@psyduk
Copy link
Author

psyduk commented Oct 16, 2013

updated with usage and documentation.

@psyduk
Copy link
Author

psyduk commented Oct 16, 2013

no problem at all. updated usage case to have an example of click event.

// var myHanlder = function () {
// $.ajax({type: 'DELETE', url: 'someurl.com', data: {id: 1}});
// };
// this.on('click', utils.once(myHandler));
Copy link
Contributor

Choose a reason for hiding this comment

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

would add this example to the docs too

@necolas
Copy link
Contributor

necolas commented Oct 16, 2013

Please could you squash your commits too (you can force push to your feature branch and this PR will be automatically updated)

@psyduk
Copy link
Author

psyduk commented Oct 16, 2013

Squashed

@necolas
Copy link
Contributor

necolas commented Oct 16, 2013

Great, thanks. Still need to move the ran = true line.

@psyduk
Copy link
Author

psyduk commented Oct 16, 2013

Oddly enough it actually broke the tests when i changed it. I'm not exactly sure on the reasoning though. I can look into why it wouldn't work.

describe('once()', function () {
it('should only call a function once', function () {
var sum = 0;
var increment = utils.once(function () { sum++; });
Copy link
Contributor

Choose a reason for hiding this comment

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

s/utils/util

@necolas
Copy link
Contributor

necolas commented Oct 17, 2013

Looks like it might have been because you referenced utils rather than util. We need to normalize it to utils in our tests at some point.

@angus-c
Copy link
Contributor

angus-c commented Oct 17, 2013

@necolas I think its fine as is no?
As long as we set ran = true after if (ran) {return} then it achieves the objective right?

@psyduk
Copy link
Author

psyduk commented Oct 17, 2013

@angus-c That seems correct. I can change it to either way.

angus-c added a commit that referenced this pull request Oct 17, 2013
@angus-c angus-c merged commit 25fe69f into flightjs:master Oct 17, 2013
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