You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by GitBox <gi...@apache.org> on 2020/12/14 09:36:36 UTC
[GitHub] [httpcomponents-core] larshelge opened a new pull request #237: Add shorthand methods for the various status classes in StatusLine
larshelge opened a new pull request #237:
URL: https://github.com/apache/httpcomponents-core/pull/237
This PR adds shorthand convenience methods for the various status classes in `StatusLine`. The purpose is to enable a more concise and shorter notation for checking the class of the HTTP response status.
The main use-case is to easily detect whether a response is a client-side or server-side error.
Currently you have to compare the status class with the inner `StatusClass` enum which is slightly awkward:
```java
if (statusLine.getStatusClass() == StatusLine.StatusClass.CLIENT_ERROR) {
// Throw a client side error
}
```
With this patch, the comparison is more concise:
`if (statusLine.is4xxClientError) {
// Throw a client side error
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] larshelge commented on pull request #237: Add shorthand methods for the various status classes in StatusLine
Posted by GitBox <gi...@apache.org>.
larshelge commented on pull request #237:
URL: https://github.com/apache/httpcomponents-core/pull/237#issuecomment-744482374
On one side there is some merit in having a common prefix to indicate those six methods being related; on the other side it will lead to some duplication/redundancy in naming. You decide.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c merged pull request #237: Add shorthand methods for the various status classes in StatusLine
Posted by GitBox <gi...@apache.org>.
ok2c merged pull request #237:
URL: https://github.com/apache/httpcomponents-core/pull/237
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] carterkozak commented on pull request #237: Add shorthand methods for the various status classes in StatusLine
Posted by GitBox <gi...@apache.org>.
carterkozak commented on pull request #237:
URL: https://github.com/apache/httpcomponents-core/pull/237#issuecomment-744743319
A common prefix would be similar to adding these methods directly to `StatusClass`, for example
```java
// original:
statusLine.isError();
// prefix proposal
// At a glance this looks like we're checking for malformed status lines
statusLine.isStatusError();
// alternative StatusClass utility functionality
statusLine.getStatusClass().isError();
```
Usage wouldn't be quite as concise as the current proposal, but it might be a better fit than extending method names.
That said, I don't have a preference one way or the other, just pointing out an alternative. The utility functionality will be helpful.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] garydgregory commented on a change in pull request #237: Add shorthand methods for the various status classes in StatusLine
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #237:
URL: https://github.com/apache/httpcomponents-core/pull/237#discussion_r542859213
##########
File path: httpcore5/src/main/java/org/apache/hc/core5/http/message/StatusLine.java
##########
@@ -100,6 +99,61 @@ public StatusClass getStatusClass() {
return this.statusClass;
}
+ /**
+ * Whether this status code is in the HTTP series {@link StatusClass#INFORMATIONAL}.
+ *
+ * @since 5.1
+ */
+ public boolean isInformational() {
+ return getStatusClass() == StatusClass.INFORMATIONAL;
+ }
+
+ /**
+ * Whether this status code is in the HTTP series {@link StatusClass#SUCCESSFUL}.
+ *
+ * @since 5.1
+ */
+ public boolean isSuccessful() {
+ return getStatusClass() == StatusClass.SUCCESSFUL;
+ }
+
+ /**
+ * Whether this status code is in the HTTP series {@link StatusClass#REDIRECTION}.
+ *
+ * @since 5.1
+ */
+ public boolean isRedirection() {
+ return getStatusClass() == StatusClass.REDIRECTION;
+ }
+
+ /**
+ * Whether this status code is in the HTTP series {@link StatusClass#CLIENT_ERROR}.
+ *
+ * @since 5.1
+ */
+ public boolean isClientError() {
+ return getStatusClass() == StatusClass.CLIENT_ERROR;
+ }
+
+ /**
+ * Whether this status code is in the HTTP series {@link StatusClass#SERVER_ERROR}.
+ *
+ * @since 5.1
+ */
+ public boolean isServerError() {
+ return getStatusClass() == StatusClass.SERVER_ERROR;
+ }
+
+ /**
+ * Whether this status code is in the HTTP series {@link StatusClass#CLIENT_ERROR}
+ * or {@link StatusClass#SERVER_ERROR}.
+ *
+ * @since 5.1
+ */
+ public boolean isError() {
+ return (isClientError() || isServerError());
Review comment:
These extra parents are not needed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] larshelge edited a comment on pull request #237: Add shorthand methods for the various status classes in StatusLine
Posted by GitBox <gi...@apache.org>.
larshelge edited a comment on pull request #237:
URL: https://github.com/apache/httpcomponents-core/pull/237#issuecomment-744482374
On one side there is some merit in having a common prefix to indicate those six methods being related; on the other side it will lead to some duplication/redundancy in naming and the "status" part is somewhat implied in the class named (as mentioned).
Added @since tags now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] larshelge commented on pull request #237: Add shorthand methods for the various status classes in StatusLine
Posted by GitBox <gi...@apache.org>.
larshelge commented on pull request #237:
URL: https://github.com/apache/httpcomponents-core/pull/237#issuecomment-744326291
I can agree with that @michael-o . I have updated the PR now. Thanks for the quick response.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] garydgregory commented on pull request #237: Add shorthand methods for the various status classes in StatusLine
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #237:
URL: https://github.com/apache/httpcomponents-core/pull/237#issuecomment-744696994
> On one side there is some merit in having a common prefix to indicate those six methods being related; on the other side it will lead to some duplication/redundancy in naming and the "status" part is somewhat implied in the class name (as mentioned).
>
> Added @ since tags now.
I would really prefer a common prefix to indicate that all of these methods are not only related but mutually exclusive, IOW only one isStatus will return true. The common prefix removed any guesswork IMO.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] michael-o commented on pull request #237: Add shorthand methods for the various status classes in StatusLine
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #237:
URL: https://github.com/apache/httpcomponents-core/pull/237#issuecomment-744438208
@garydgregory Why? The class name says `StatusLine`. Does it make sense to duplicate part of that name into the method names?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] larshelge commented on a change in pull request #237: Add shorthand methods for the various status classes in StatusLine
Posted by GitBox <gi...@apache.org>.
larshelge commented on a change in pull request #237:
URL: https://github.com/apache/httpcomponents-core/pull/237#discussion_r543116067
##########
File path: httpcore5/src/main/java/org/apache/hc/core5/http/message/StatusLine.java
##########
@@ -100,6 +99,61 @@ public StatusClass getStatusClass() {
return this.statusClass;
}
+ /**
+ * Whether this status code is in the HTTP series {@link StatusClass#INFORMATIONAL}.
+ *
+ * @since 5.1
+ */
+ public boolean isInformational() {
+ return getStatusClass() == StatusClass.INFORMATIONAL;
+ }
+
+ /**
+ * Whether this status code is in the HTTP series {@link StatusClass#SUCCESSFUL}.
+ *
+ * @since 5.1
+ */
+ public boolean isSuccessful() {
+ return getStatusClass() == StatusClass.SUCCESSFUL;
+ }
+
+ /**
+ * Whether this status code is in the HTTP series {@link StatusClass#REDIRECTION}.
+ *
+ * @since 5.1
+ */
+ public boolean isRedirection() {
+ return getStatusClass() == StatusClass.REDIRECTION;
+ }
+
+ /**
+ * Whether this status code is in the HTTP series {@link StatusClass#CLIENT_ERROR}.
+ *
+ * @since 5.1
+ */
+ public boolean isClientError() {
+ return getStatusClass() == StatusClass.CLIENT_ERROR;
+ }
+
+ /**
+ * Whether this status code is in the HTTP series {@link StatusClass#SERVER_ERROR}.
+ *
+ * @since 5.1
+ */
+ public boolean isServerError() {
+ return getStatusClass() == StatusClass.SERVER_ERROR;
+ }
+
+ /**
+ * Whether this status code is in the HTTP series {@link StatusClass#CLIENT_ERROR}
+ * or {@link StatusClass#SERVER_ERROR}.
+ *
+ * @since 5.1
+ */
+ public boolean isError() {
+ return (isClientError() || isServerError());
Review comment:
Fixed now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] michael-o commented on pull request #237: Add shorthand methods for the various status classes in StatusLine
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #237:
URL: https://github.com/apache/httpcomponents-core/pull/237#issuecomment-745141832
Looks fine for me now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] michael-o commented on pull request #237: Add shorthand methods for the various status classes in StatusLine
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #237:
URL: https://github.com/apache/httpcomponents-core/pull/237#issuecomment-744723516
> > On one side there is some merit in having a common prefix to indicate those six methods being related; on the other side it will lead to some duplication/redundancy in naming and the "status" part is somewhat implied in the class name (as mentioned).
> > Added @ since tags now.
>
> I would really prefer a common prefix to indicate that all of these methods are not only related but mutually exclusive, IOW only one isStatus will return true. The common prefix removed any guesswork IMO.
Why can't we simply document this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] larshelge edited a comment on pull request #237: Add shorthand methods for the various status classes in StatusLine
Posted by GitBox <gi...@apache.org>.
larshelge edited a comment on pull request #237:
URL: https://github.com/apache/httpcomponents-core/pull/237#issuecomment-744482374
On one side there is some merit in having a common prefix to indicate those six methods being related; on the other side it will lead to some duplication/redundancy in naming and the "status" part is somewhat implied in the class name (as mentioned).
Added @ since tags now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] garydgregory commented on pull request #237: Add shorthand methods for the various status classes in StatusLine
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #237:
URL: https://github.com/apache/httpcomponents-core/pull/237#issuecomment-744743448
You are correct, I lost sight that this code is in the status line class, I thought it was in a class that held a status line instance. Please disregard!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] larshelge edited a comment on pull request #237: Add shorthand methods for the various status classes in StatusLine
Posted by GitBox <gi...@apache.org>.
larshelge edited a comment on pull request #237:
URL: https://github.com/apache/httpcomponents-core/pull/237#issuecomment-744482374
On one side there is some merit in having a common prefix to indicate those six methods being related; on the other side it will lead to some duplication/redundancy in naming and the "status" part is somewhat implied in the class named (as mentioned).
Added @ since tags now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] larshelge edited a comment on pull request #237: Add shorthand methods for the various status classes in StatusLine
Posted by GitBox <gi...@apache.org>.
larshelge edited a comment on pull request #237:
URL: https://github.com/apache/httpcomponents-core/pull/237#issuecomment-744326291
I can agree with that @michael-o. I have updated the PR now. Thanks for the quick response.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] garydgregory commented on pull request #237: Add shorthand methods for the various status classes in StatusLine
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #237:
URL: https://github.com/apache/httpcomponents-core/pull/237#issuecomment-744434261
It seems to me that all the new methods should have the same prefix, not just "is" but maybe "isStatus". Also the public methods should have "@since" Javadoc tags.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org