refactor: Optimize Java Implementation#1545
Conversation
product-auto-label
bot
added
size: m
product-auto-label
bot
added
size: l
MarkDuckworth
left a comment
MarkDuckworth
left a comment
There was a problem hiding this comment.
LGTM with comments for your consideration
| /** Returns the number of path components. */ | ||
| int size() { | ||
| return this.getSegments().size(); | ||
| return Integer.compare(thisSegments.size(), otherSegments.size()); |
There was a problem hiding this comment.
nit: in the spirit of optimization, you can compare size first before iterating path segments and comparing strings.
There was a problem hiding this comment.
That would work for equals, but compare needs to determine order.
|
|
||
| /** A Firestore BulkWriter that can be used to perform a large number of writes in parallel. */ | ||
| @BetaApi | ||
| public final class BulkWriter implements AutoCloseable { |
There was a problem hiding this comment.
Bringing BulkWriter out of beta, is that worthy of a minor version bump in the commit message
feat: BulkWriter out of beta
| } | ||
| FieldOrder filter = (FieldOrder) o; | ||
| return Objects.equals(toProto(), filter.toProto()); | ||
| if (direction != filter.direction) return false; |
There was a problem hiding this comment.
ultraNit. May as well use brackets consistently with the other conditionals.
tom-andersen
added
the
owlbot:run
gcf-owl-bot
bot
removed
the
owlbot:run
tom-andersen
added
the
automerge
|
Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot. |
gcf-merge-on-green
bot
removed
the
automerge
tom-andersen
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
3 participants
Optimizations:
builderWithExpectedSizewhen possible.Preconditions.checkArgumentto avoid building strings unless required.SortedMaporImmutableListwas required, but a different type was originally created.equalsmethod. Instead, compare values directly.QueryDocumentSnapshotcomparator. We should avoid creating objects as part of comparison.String.joininstead of rolling our own.compareTowhen==.