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

ndk/media_codec: Inline get_unlikely_to_be_null() #400

Merged
merged 1 commit into from
Jun 15, 2023

Conversation

MarijnS95
Copy link
Member

Back when I refactored these in #248 and #256 the closure argument should have been removed entirely as it is only called immediately without any arguments, so its return value could have been passed instead. At that point the function becomes simple enough to warrant removal entirely, inlining the unwrap and even allowing a more natural string literal error message in .expect().

CC @zarik5

Back when I refactored these in #248 and #256 the closure argument
should have been removed entirely as it is only called immediately
without any arguments, so its return value could have been passed
instead.  At that point the function becomes simple enough to warrant
removal entirely, inlining the unwrap and even allowing a more natural
string literal error message in `.expect()`.
@zarik5
Copy link
Contributor

zarik5 commented Jun 15, 2023

I agree this is an improvement. The function name get_unlikely_to_be_null was a bit complex and obscure, that's usually a sign the function should be broken down or removed.

@MarijnS95
Copy link
Member Author

At least it's descriptive, which can't be said for the other functions (does it "assume" this is never null? Or validate that it isn't?).

@MarijnS95 MarijnS95 merged commit 4da2e5b into master Jun 15, 2023
@MarijnS95 MarijnS95 deleted the ndk-media-codec-inline-nonnull-getter branch June 15, 2023 17:44
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.

2 participants