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

improved try documentation #17378

Closed
wants to merge 1 commit into from
Closed

improved try documentation #17378

wants to merge 1 commit into from

Conversation

egilburg
Copy link
Contributor

  • better if example
  • Added chaining example to the try method description
  • Documented the respond_to? check to the try method description
  • Clearer wording to explain that argument error is raised on argument mismatch to responding method, rather than to non-responding method (which is handled without exception by try)
  • .any? is more precise than ! .blank?
    • Don't need to use try on children as (for regular associations) they will always be a collection or array that responds to first
  • Fix typos/grammar

- better `if` example
- Added chaining example to the try method description
- Documented the `respond_to?` check to the try method description
- Clearer wording to explain that argument error is raised on argument mismatch to responding method, rather than to non-responding method (which is handled without exception by `try`)
- `.any?` is more precise than `! .blank?`
 - Don't need to use `try` on `children` as (for regular associations) they will always be a collection or array that responds to `first` 
- Fix typos/grammar
#
# +try+ will also return +nil+ if the receiver does not respond to the method:
#
# @person.try(:non_existing_method)
Copy link
Member

Choose a reason for hiding this comment

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

Please show the return value

zzak pushed a commit to zzak/rails that referenced this pull request Oct 24, 2014
@zzak
Copy link
Member

zzak commented Oct 24, 2014

Merged manually in df1dda8

@zzak zzak closed this Oct 24, 2014
#
# With +try+
# @person.try(:children).try(:first).try(:name)
# @person.try(:children).first.try(:name)
Copy link
Member

Choose a reason for hiding this comment

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

@egilburg why was this changed?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad I misread the code :-( I'll make a fix pr soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops nvm I see you already reverted it, thanks!

Copy link
Member

Choose a reason for hiding this comment

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

@chancancode @egilburg I missed this sorry, thanks for fixing it!

Copy link
Member

Choose a reason for hiding this comment

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

No problem, thanks all! 😄

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