Skip to content

[#2286]: Underscore could use a _.propertyResult #2531

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[#2286]: Underscore could use a _.propertyResult #2531

wants to merge 1 commit into from

Conversation

monkpow
Copy link

@monkpow monkpow commented May 5, 2016 •
edited by megawac
Loading

I chose the name _.method per the above suggestion, but am happy to adjust if this does not match the project guidelines.

Fixes #2286

@coveralls
Copy link

coveralls commented May 5, 2016 •
edited
Loading

Coverage Status

Coverage increased (+0.03%) to 96.894% when pulling 10bc63e on monkpow:master into 53086b0 on jashkenas:master.

megawac
megawac reviewed May 5, 2016
View reviewed changes
underscore.js Outdated
// referenced object.
_.method = function(value) {
var isFunction = this.isFunction,
rest = this.rest,
Copy link
Collaborator

Choose a reason for hiding this comment

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

use restArgs instead

@coveralls
Copy link

coveralls commented May 5, 2016 •
edited
Loading

Coverage Status

Coverage increased (+0.03%) to 96.894% when pulling 037d5a8 on monkpow:master into 53086b0 on jashkenas:master.

@monkpow
Copy link
Author

monkpow commented May 5, 2016

@megawac Thanks for the quality feedback. Updated diff addresses:

  • use restArgs
  • don't use this keyword, use direct reference
  • Clarified comment and token in response to Question: "is this not valid when a function is provided?". Added narrative above in this thread.
  • check if value is a function in the outer function.

megawac
megawac reviewed May 5, 2016
View reviewed changes
underscore.js Outdated

return restArgs(function(obj, args) {
if (obj == null) { return; }
func = isFunctionLiteral ? value : obj[value];
Copy link
Collaborator

Choose a reason for hiding this comment

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

define func here rather than outer scope

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, updated diff with this change.

@jdalton
Copy link
Contributor

jdalton commented May 5, 2016

fyi lodash/method.

Add _.method as peer to _.property for capturing return values of fun…
3fa1ae3 
…ctions on an Object. Closes jashkenas#2286: Underscore could use a _.propertyResult.
@coveralls
Copy link

coveralls commented May 6, 2016 •
edited
Loading

Coverage Status

Coverage increased (+0.02%) to 96.891% when pulling 3fa1ae3 on monkpow:master into 53086b0 on jashkenas:master.

@jgonggrijp jgonggrijp added the contrib probably belongs in Underscore-contrib label Jan 11, 2021
@jgonggrijp jgonggrijp self-assigned this Jan 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jgonggrijp jgonggrijp

Labels
contrib probably belongs in Underscore-contrib enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Underscore could use a _.propertyResult
5 participants
@monkpow @coveralls @jdalton @megawac @jgonggrijp