Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Add Disposable.isDisposable#15

Merged
as-cii merged 2 commits intomasterfrom
as-add-isdisposable
Aug 24, 2015
Merged

Add Disposable.isDisposable#15
as-cii merged 2 commits intomasterfrom
as-add-isdisposable

Conversation

@as-cii
Copy link
Copy Markdown
Contributor

@as-cii as-cii commented Aug 24, 2015

So that we can check whether an object correctly implements the Disposable contract, as discussed in #12.

/cc: @maxbrunsfeld @nathansobo @kevinsawicki

So that we can check whether an object correctly implements the
`Disposable` contract.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Usually we don't denote these class methods w/ the ::, because that's coffee-script notation for referring to properties on a prototype.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I had a look at how we did it in text-buffer and I mistakenly assumed it was a standard to refer to every kind of method.

Thanks for pointing this out! 👍

@maxbrunsfeld
Copy link
Copy Markdown
Contributor

Other than the one comment, looks good to me! Thanks for doing this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the existential operator needed here? It might be useful to blow up if you're accidentally passing in a null or undefined object

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Generally, I could go either way, but I think it's good to imitate Array.isArray, Number.isNaN et al in this case. They return false when passed null or undefined.

@maxbrunsfeld
Copy link
Copy Markdown
Contributor

🚢

@as-cii as-cii changed the title Add Disposable::isDisposable Add Disposable.isDisposable Aug 24, 2015
as-cii added a commit that referenced this pull request Aug 24, 2015
@as-cii as-cii merged commit 5a64620 into master Aug 24, 2015
@as-cii as-cii deleted the as-add-isdisposable branch August 24, 2015 16:30
@as-cii
Copy link
Copy Markdown
Contributor Author

as-cii commented Aug 24, 2015

❤️

@nathansobo
Copy link
Copy Markdown
Contributor

I like this approach. Less obtrusive.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants