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

Replace typeof(X).Name with nameof(X) #1077 #1091

Merged

Conversation

manognya-b
Copy link
Contributor

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a change, please open an issue to discuss the change or find an existing issue.

Replaced all occurrences of typeof(X).Name with nameof(X)

Fixes (#1077)

Description

Replaced usages of typeof(X).Name with nameof(X) across the codebase to improve compile-time safety and readability.
The nameof(X) expression ensures the type name is determined at compile time rather than runtime, preventing runtime-related issues and potentially improving performance. This change aligns with modern C# practices and simplifies future maintenance by eliminating unnecessary runtime operations.

* Replaced all occurrences of typeof(X).Name with nameof(X)

* Improved code readability and compile-time safety
@paulirwin paulirwin self-requested a review January 6, 2025 22:04
Copy link
Contributor

@paulirwin paulirwin left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! It mostly looks good. There are a couple cases we'll have to revert that I didn't consider until now, where we can't use this on generic type parameters. The rest of the comments are about string interpolation and some const conversions. Should be pretty quick to fix. If you are unable to make the changes, let me know and I'd be happy to do them.

@paulirwin paulirwin added the notes:improvement An enhancement to an existing feature label Jan 6, 2025
@manognya-b
Copy link
Contributor Author

Thank you for reviewing my PR! I've taken your comments into consideration and made the required changes, hopefully the code looks good now. The cases where I had to revert were very interesting and I appreciated the insights into why each change was requested as it gave me more context into the workings of runtime behaviour for those specific functionalities. Open to making more changes if required!

@paulirwin paulirwin merged commit c2583f0 into apache:master Jan 7, 2025
267 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notes:improvement An enhancement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants