Skip to content

Return null on GET 404#1206

Merged
martinsawicki merged 1 commit into
Azure:masterfrom
jianghaolu:404null
Oct 19, 2016
Merged

Return null on GET 404#1206
martinsawicki merged 1 commit into
Azure:masterfrom
jianghaolu:404null

Conversation

@jianghaolu

@jianghaolu jianghaolu commented Oct 19, 2016

Copy link
Copy Markdown
Contributor

Current behavior is throwing CloudException. Changed to simply return null. Related PR: Azure/autorest-clientruntime-for-java#123


@Override
protected BatchAccountImpl wrapModel(BatchAccountInner inner) {
if (inner == null) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

might be simpler/cleaner to just say:
return (inner == null) ? inner : new .... ;


@Override
protected BatchAccountImpl wrapModel(BatchAccountInner inner) {
if (inner == null) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

might be simpler/cleaner to just say:
return (inner == null) ? null : new .... ;

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.

I actually find inline conditionals hard to read, particularly in this case the line gets too long with the inline constructor call. Inline conditionals are forbidden in Sun's original checkstyle rules: http://checkstyle.sourceforge.net/config_coding.html#AvoidInlineConditionals but we are not reinforcing it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ok

@martinsawicki martinsawicki merged commit 7aaba7b into Azure:master Oct 19, 2016
jianghaolu pushed a commit to jianghaolu/azure-sdk-for-java that referenced this pull request Feb 27, 2017
sima-zhu pushed a commit to sima-zhu/azure-sdk-for-java that referenced this pull request Mar 21, 2019
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.

3 participants