Skip to content
This repository has been archived by the owner on Sep 28, 2021. It is now read-only.

Make comparing status more convenient. #187

Merged
merged 2 commits into from
May 22, 2017
Merged

Conversation

rydenius
Copy link
Contributor

More than once I have seen code like this fail:

if (Status.OK == response.status()) {

or

if (Status.OK.equals(response.status()) {

because the status of the response has a different reason phrase. The current solution would be to use

if (Status.OK.code() == response.status().code()) {

This small patch would simplify this check and also signal to developers that regular equals method might not be what you want to use.

@danielstahl
Copy link

👍

1 similar comment
@martina-if
Copy link
Contributor

👍

@codecov-io
Copy link

codecov-io commented Dec 21, 2016

Codecov Report

Merging #187 into master will increase coverage by 0.08%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master     #187      +/-   ##
============================================
+ Coverage     69.95%   70.04%   +0.08%     
- Complexity        0      681     +681     
============================================
  Files           143      143              
  Lines          2949     2951       +2     
  Branches        239      171      -68     
============================================
+ Hits           2063     2067       +4     
+ Misses          840      838       -2     
  Partials         46       46
Impacted Files Coverage Δ Complexity Δ
...i/src/main/java/com/spotify/apollo/StatusType.java 47.82% <100%> (+9.73%) 4 <4> (+4)
...o-api/src/main/java/com/spotify/apollo/Status.java 84.84% <0%> (+1.51%) 6% <0%> (+6%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b0eeb5...7d1b306. Read the comment docs.

@bridell
Copy link

bridell commented Dec 21, 2016

👍

@mattnworb
Copy link
Member

👍 I've made this mistake often

* @return true if code of this equals code of statusType
*/
default boolean equalCode(StatusType statusType) {
return statusType != null && code() == statusType.code();
Copy link
Contributor

Choose a reason for hiding this comment

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

Would rather have this throw NPE... Fail early!

Copy link
Member

Choose a reason for hiding this comment

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

I would agree, I think. @rydenius, is there a reason to expect/permit users to pass in a null statusType to this method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed now.

* @return true if family of this equals family of statusType
*/
default boolean equalFamily(StatusType statusType) {
return statusType != null && family() == statusType.family();
Copy link
Contributor

Choose a reason for hiding this comment

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

Same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed now.

@martina-if martina-if merged commit 9e37c4b into spotify:master May 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants