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

Website and Guide only: Add try/finally example (closing Response) #3097

Closed
wants to merge 2 commits into from

Conversation

rbygrave
Copy link
Contributor

@rbygrave rbygrave commented Jan 9, 2017

Note this example uses okhttp3.internal.Util.closeQuietly() ... to close potentially null Response. I'm not sure if you want to encourage that or not.

@@ -107,6 +107,33 @@ <h3 id="examples">Examples</h3>
}
</pre>

<h4>Try/Finally example</h4>
Copy link
Member

Choose a reason for hiding this comment

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

I think this might be a distraction from our otherwise spartan docs. Wanna just fix the first example to use try/finally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can do that. It does mean the very first example users see is ... a bit 'irk' imo. It is completely your call though ... (I'm heading home now, I'll check in later).

/**
* Example using try/finally (typically when try with resources is not available).
*/
public class GetTryFinallyExample {
Copy link
Member

Choose a reason for hiding this comment

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

This might be overkill.

@swankjesse
Copy link
Member

Sorry for sending you in circles on this. I really want our docs to show off OkHttp’s API, and not tour the madness that is writing defensive Java code. Maybe the standalone try block is sufficient.

Gross sample code is gross.

@yschimke
Copy link
Collaborator

Closing after a year

@yschimke yschimke closed this Feb 23, 2018
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