Skip to content

Cleanup a number of warnings in the code. #137

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
Dec 4, 2018
Merged

Cleanup a number of warnings in the code. #137

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ca50431
Eliminate 'badimport' warning
rsgowman Nov 21, 2018
6b94249
Suppress EqualsGetClass warnings for public classes
rsgowman Nov 21, 2018
f72a5b3
Mark a number of classes final
rsgowman Nov 21, 2018
8a98285
Switch .getClass() to instanceof in some .equals() methods
rsgowman Nov 21, 2018
23e5f94
Provide toString on FieldMask
rsgowman Nov 21, 2018
917b0f8
Fix a 'missing override' warning
rsgowman Nov 22, 2018
34e8b0e
Rework FieldMask to use a Set of FieldPaths rather than a Collection.
rsgowman Nov 22, 2018
024e9fe
Review feedback
rsgowman Nov 23, 2018
4d88c95
Review feedback
rsgowman Nov 27, 2018
497434f
Add TestUtil.fieldMask(String...) to simplify creating FieldMasks.
rsgowman Nov 28, 2018
124db7f
googleJavaFormat
rsgowman Nov 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions firebase-firestore/src/main/java/com/google/firebase/firestore/DocumentReference.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,7 @@ private ListenerRegistration addSnapshotListenerInternal(
}

@Override
@SuppressWarnings("EqualsGetClass") // This class is effectively final.
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: why not use instanceof here and in Query#equals? The article you linked states that instanceof is never worse than getClass:

What about final or effectively-final classes?

In this case there is no disadvantage to the getClass trick - but there’s no great advantage to it either. Most unsafe idioms have circumstances in which they are safe, but this doesn’t change the fact that they are generally unsafe and not worth propagating and legitimizing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, done.

public boolean equals(Object o) {
if (this == o) {
return true;
Expand Down
1 change: 1 addition & 0 deletions firebase-firestore/src/main/java/com/google/firebase/firestore/Query.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,7 @@ private ListenerRegistration addSnapshotListenerInternal(
}

@Override
@SuppressWarnings("EqualsGetClass") // This class is effectively final.
public boolean equals(Object o) {
if (this == o) {
return true;
Expand Down