Skip to content

[AST] rename some DeclContext isXXX methods to getAsXXX #1279

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

Merged
merged 1 commit into from
Feb 12, 2016

Conversation

dduan
Copy link
Contributor

@dduan dduan commented Feb 11, 2016

There's a group of methods in DeclContext with names that start with is,
such as isClassOrClassExtensionContext(). These names suggests a boolean
return value, while the methods actually return a type declaration. This
patch replaces the is prefix with get to better reflect their interface.

(build-script -RT passes on OS X).

@dduan dduan changed the title [AST] rename some isXXX methods to getXXX [AST] rename some DeclContext isXXX methods to getXXX Feb 11, 2016
@nkcsgexi
Copy link
Contributor

I think the original name is beneficial. Using getXXX suggests part-of-whole relationship, which is not true here in DeclContext.

@nkcsgexi
Copy link
Contributor

Probably a better alternative is getAsXXX().

@dduan
Copy link
Contributor Author

dduan commented Feb 11, 2016

@nkcsgexi I agree that getXXX suggests part-of-whole relationship.

But getXXX is a convention used through out Swift AST (see example 1 and 2). So there's a consistency argument to be made here.

@lattner
Copy link
Contributor

lattner commented Feb 11, 2016

I'd also prefer getAsXXX for things that can fail/return null.

@nkcsgexi
Copy link
Contributor

I am not convinced that being consistent is a concern here. All we need to care is to convey the right message.

There's a group of methods in `DeclContext` with names that start with *is*,
such as `isClassOrClassExtensionContext()`. These names suggests a boolean
return value, while the methods actually return a type declaration. This
patch replaces the *is* prefix with *getAs* to better reflect their interface.
@dduan dduan force-pushed the rename_decl_context_methods branch from 86305dd to efe2307 Compare February 12, 2016 00:24
@dduan
Copy link
Contributor Author

dduan commented Feb 12, 2016

update: get -> getAs

@dduan dduan changed the title [AST] rename some DeclContext isXXX methods to getXXX [AST] rename some DeclContext isXXX methods to getAsXXX Feb 12, 2016
nkcsgexi added a commit that referenced this pull request Feb 12, 2016
[AST] rename some DeclContext isXXX methods to getAsXXX
@nkcsgexi nkcsgexi merged commit f7def54 into swiftlang:master Feb 12, 2016
@nkcsgexi
Copy link
Contributor

Cool! Thanks!

dduan pushed a commit to dduan/swift that referenced this pull request Feb 12, 2016
[AST] rename some DeclContext isXXX methods to getAsXXX
@dduan dduan deleted the rename_decl_context_methods branch December 23, 2019 21:48
MaxDesiatov pushed a commit that referenced this pull request Oct 19, 2020
[pull] swiftwasm from master
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.

4 participants