Skip to content

Commit db3e8e7

Browse files
committed
Use isEmpty() Instead of size(); toList()
Sonar: Collection.isEmpty() should be used to test for emptiness (java:S1155) CODE_SMELL_MINOR Using Collection.size() to test for emptiness works, but using Collection.isEmpty() makes the code more readable and can be more performant. The time complexity of any isEmpty() method implementation should be O(1) whereas some implementations of size() can be O(n). Also "Stream.toList()" method should be used instead of "collectors" when unmodifiable list needed (java:S6204) CODE_SMELL_MAJOR In Java 8 Streams were introduced to support chaining of operations over collections in a functional style. The most common way to save a result of such chains is to save them to some collection (usually List). To do so there is a terminal method collect that can be used with a library of Collectors. The key problem is that `.collect(Collectors.toList())` actually returns a mutable kind of List while in the majority of cases unmodifiable lists are preferred. In Java 10 a new collector appeared to return an unmodifiable list: `toUnmodifiableList()`. This does the trick but results in verbose code. Since Java 16 there is now a better variant to produce an unmodifiable list directly from a stream: Stream.toList().
1 parent e3f9c85 commit db3e8e7

File tree

1 file changed

+29
-28
lines changed

1 file changed

+29
-28
lines changed

spring-kafka/src/main/java/org/springframework/kafka/listener/KafkaMessageListenerContainer.java

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1428,7 +1428,7 @@ protected void pollAndInvoke() {
14281428
doProcessCommits();
14291429
fixTxOffsetsIfNeeded();
14301430
idleBetweenPollIfNecessary();
1431-
if (this.seeks.size() > 0) {
1431+
if (!this.seeks.isEmpty()) {
14321432
processSeeks();
14331433
}
14341434
pauseConsumerIfNecessary();
@@ -1586,7 +1586,7 @@ private void fixTxOffsetsIfNeeded() {
15861586
toFix.put(tp, createOffsetAndMetadata(position));
15871587
}
15881588
});
1589-
if (toFix.size() > 0) {
1589+
if (!toFix.isEmpty()) {
15901590
this.logger.debug(() -> "Fixing TX offsets: " + toFix);
15911591
if (this.kafkaTxManager == null) {
15921592
if (this.syncCommits) {
@@ -1689,7 +1689,7 @@ private synchronized void captureOffsets(ConsumerRecords<K, V> records) {
16891689
}
16901690

16911691
private void checkRebalanceCommits() {
1692-
if (this.commitsDuringRebalance.size() > 0) {
1692+
if (!this.commitsDuringRebalance.isEmpty()) {
16931693
// Attempt to recommit the offsets for partitions that we still own
16941694
Map<TopicPartition, OffsetAndMetadata> commits = this.commitsDuringRebalance.entrySet()
16951695
.stream()
@@ -1729,7 +1729,8 @@ private void debugRecords(@Nullable ConsumerRecords<K, V> records) {
17291729
.flatMap(p -> records.records(p).stream())
17301730
// map to same format as send metadata toString()
17311731
.map(r -> r.topic() + "-" + r.partition() + "@" + r.offset())
1732-
.collect(Collectors.toList()).toString());
1732+
.toList()
1733+
.toString());
17331734
}
17341735
}
17351736
}
@@ -1755,11 +1756,11 @@ private void pauseConsumerIfNecessary() {
17551756
}
17561757

17571758
private void doPauseConsumerIfNecessary() {
1758-
if (this.pausedForNack.size() > 0) {
1759+
if (!this.pausedForNack.isEmpty()) {
17591760
this.logger.debug("Still paused for nack sleep");
17601761
return;
17611762
}
1762-
if (this.offsetsInThisBatch != null && this.offsetsInThisBatch.size() > 0 && !this.pausedForAsyncAcks) {
1763+
if (this.offsetsInThisBatch != null && !this.offsetsInThisBatch.isEmpty() && !this.pausedForAsyncAcks) {
17631764
this.pausedForAsyncAcks = true;
17641765
this.logger.debug(() -> "Pausing for incomplete async acks: " + this.offsetsInThisBatch);
17651766
}
@@ -1800,7 +1801,7 @@ else if (this.offsetsInThisBatch != null) {
18001801
}
18011802

18021803
private void doResumeConsumerIfNeccessary() {
1803-
if (this.pausedForAsyncAcks && this.offsetsInThisBatch.size() == 0) {
1804+
if (this.pausedForAsyncAcks && this.offsetsInThisBatch.isEmpty()) {
18041805
this.pausedForAsyncAcks = false;
18051806
this.logger.debug("Resuming after manual async acks cleared");
18061807
}
@@ -1822,8 +1823,8 @@ private void pausePartitionsIfNecessary() {
18221823
.stream()
18231824
.filter(tp -> isPartitionPauseRequested(tp)
18241825
&& !pausedConsumerPartitions.contains(tp))
1825-
.collect(Collectors.toList());
1826-
if (partitionsToPause.size() > 0) {
1826+
.toList();
1827+
if (!partitionsToPause.isEmpty()) {
18271828
this.consumer.pause(partitionsToPause);
18281829
this.pausedPartitions.addAll(partitionsToPause);
18291830
this.logger.debug(() -> "Paused consumption from " + partitionsToPause);
@@ -1839,8 +1840,8 @@ private void resumePartitionsIfNecessary() {
18391840
.stream()
18401841
.filter(tp -> !isPartitionPauseRequested(tp)
18411842
&& this.pausedPartitions.contains(tp))
1842-
.collect(Collectors.toList());
1843-
if (partitionsToResume.size() > 0) {
1843+
.toList();
1844+
if (!partitionsToResume.isEmpty()) {
18441845
this.consumer.resume(partitionsToResume);
18451846
this.pausedPartitions.removeAll(partitionsToResume);
18461847
this.logger.debug(() -> "Resumed consumption from " + partitionsToResume);
@@ -1876,7 +1877,7 @@ private void checkIdle() {
18761877
private void idleBetweenPollIfNecessary() {
18771878
long idleBetweenPolls = this.containerProperties.getIdleBetweenPolls();
18781879
Collection<TopicPartition> assigned = getAssignedPartitions();
1879-
if (idleBetweenPolls > 0 && assigned != null && assigned.size() > 0) {
1880+
if (idleBetweenPolls > 0 && assigned != null && !assigned.isEmpty()) {
18801881
idleBetweenPolls = Math.min(idleBetweenPolls,
18811882
this.maxPollInterval - (System.currentTimeMillis() - this.lastPoll)
18821883
- 5000); // NOSONAR - less by five seconds to avoid race condition with rebalance
@@ -1959,7 +1960,7 @@ protected void handleConsumerException(Exception e) {
19591960

19601961
private void commitPendingAcks() {
19611962
processCommits();
1962-
if (this.offsets.size() > 0) {
1963+
if (!this.offsets.isEmpty()) {
19631964
// we always commit after stopping the invoker
19641965
commitIfNecessary();
19651966
}
@@ -2055,19 +2056,19 @@ private synchronized void ackInOrder(ConsumerRecord<K, V> record) {
20552056
TopicPartition part = new TopicPartition(record.topic(), record.partition());
20562057
List<Long> offs = this.offsetsInThisBatch.get(part);
20572058
List<ConsumerRecord<K, V>> deferred = this.deferredOffsets.get(part);
2058-
if (offs.size() > 0) {
2059+
if (!offs.isEmpty()) {
20592060
if (offs.get(0) == record.offset()) {
20602061
offs.remove(0);
20612062
ConsumerRecord<K, V> recordToAck = record;
2062-
if (deferred.size() > 0) {
2063+
if (!deferred.isEmpty()) {
20632064
Collections.sort(deferred, (a, b) -> Long.compare(a.offset(), b.offset()));
2064-
while (deferred.size() > 0 && deferred.get(0).offset() == recordToAck.offset() + 1) {
2065+
while (!deferred.isEmpty() && deferred.get(0).offset() == recordToAck.offset() + 1) {
20652066
recordToAck = deferred.remove(0);
20662067
offs.remove(0);
20672068
}
20682069
}
20692070
processAck(recordToAck);
2070-
if (offs.size() == 0) {
2071+
if (offs.isEmpty()) {
20712072
this.deferredOffsets.remove(part);
20722073
this.offsetsInThisBatch.remove(part);
20732074
}
@@ -2147,7 +2148,7 @@ private void invokeBatchListener(final ConsumerRecords<K, V> recordsArg) {
21472148
if (!this.wantsFullRecords) {
21482149
recordList = createRecordList(records);
21492150
}
2150-
if (this.wantsFullRecords || recordList.size() > 0) {
2151+
if (this.wantsFullRecords || !recordList.isEmpty()) {
21512152
if (this.transactionTemplate != null) {
21522153
invokeBatchListenerInTx(records, recordList); // NOSONAR
21532154
}
@@ -2624,7 +2625,7 @@ private boolean checkImmediatePause(Iterator<ConsumerRecord<K, V>> iterator) {
26242625
remaining.computeIfAbsent(new TopicPartition(next.topic(), next.partition()),
26252626
tp -> new ArrayList<ConsumerRecord<K, V>>()).add(next);
26262627
}
2627-
if (remaining.size() > 0) {
2628+
if (!remaining.isEmpty()) {
26282629
this.remainingRecords = new ConsumerRecords<>(remaining);
26292630
return true;
26302631
}
@@ -2692,7 +2693,7 @@ private void handleNack(final ConsumerRecords<K, V> records, final ConsumerRecor
26922693
Iterator<ConsumerRecord<K, V>> iterator2 = records.iterator();
26932694
while (iterator2.hasNext()) {
26942695
ConsumerRecord<K, V> next = iterator2.next();
2695-
if (list.size() > 0 || recordsEqual(record, next)) {
2696+
if (!list.isEmpty() || recordsEqual(record, next)) {
26962697
list.add(next);
26972698
}
26982699
}
@@ -2907,7 +2908,7 @@ private void invokeErrorHandler(final ConsumerRecord<K, V> record,
29072908
records.computeIfAbsent(new TopicPartition(next.topic(), next.partition()),
29082909
tp -> new ArrayList<ConsumerRecord<K, V>>()).add(next);
29092910
}
2910-
if (records.size() > 0) {
2911+
if (!records.isEmpty()) {
29112912
this.remainingRecords = new ConsumerRecords<>(records);
29122913
this.pauseForPending = true;
29132914
}
@@ -3179,10 +3180,10 @@ private void initPartitionsIfNeeded() {
31793180
private void doInitialSeeks(Map<TopicPartition, OffsetMetadata> partitions, Set<TopicPartition> beginnings,
31803181
Set<TopicPartition> ends) {
31813182

3182-
if (beginnings.size() > 0) {
3183+
if (!beginnings.isEmpty()) {
31833184
this.consumer.seekToBeginning(beginnings);
31843185
}
3185-
if (ends.size() > 0) {
3186+
if (!ends.isEmpty()) {
31863187
this.consumer.seekToEnd(ends);
31873188
}
31883189
for (Entry<TopicPartition, OffsetMetadata> entry : partitions.entrySet()) {
@@ -3311,7 +3312,7 @@ public void seekToBeginning(String topic, int partition) {
33113312
public void seekToBeginning(Collection<TopicPartition> partitions) {
33123313
this.seeks.addAll(partitions.stream()
33133314
.map(tp -> new TopicPartitionOffset(tp.topic(), tp.partition(), SeekPosition.BEGINNING))
3314-
.collect(Collectors.toList()));
3315+
.toList());
33153316
}
33163317

33173318
@Override
@@ -3323,7 +3324,7 @@ public void seekToEnd(String topic, int partition) {
33233324
public void seekToEnd(Collection<TopicPartition> partitions) {
33243325
this.seeks.addAll(partitions.stream()
33253326
.map(tp -> new TopicPartitionOffset(tp.topic(), tp.partition(), SeekPosition.END))
3326-
.collect(Collectors.toList()));
3327+
.toList());
33273328
}
33283329

33293330
@Override
@@ -3563,15 +3564,15 @@ private void repauseIfNeeded(Collection<TopicPartition> partitions) {
35633564
toRepause.add(tp);
35643565
}
35653566
});
3566-
if (!ListenerConsumer.this.consumerPaused && toRepause.size() > 0) {
3567+
if (!ListenerConsumer.this.consumerPaused && !toRepause.isEmpty()) {
35673568
ListenerConsumer.this.consumer.pause(toRepause);
35683569
ListenerConsumer.this.logger.debug(() -> "Paused consumption from: " + toRepause);
35693570
publishConsumerPausedEvent(toRepause, "Re-paused after rebalance");
35703571
}
35713572
this.revoked.removeAll(toRepause);
35723573
ListenerConsumer.this.pausedPartitions.removeAll(this.revoked);
35733574
this.revoked.clear();
3574-
if (ListenerConsumer.this.pausedForNack.size() > 0) {
3575+
if (!ListenerConsumer.this.pausedForNack.isEmpty()) {
35753576
ListenerConsumer.this.consumer.pause(ListenerConsumer.this.pausedForNack);
35763577
}
35773578
}
@@ -3596,7 +3597,7 @@ private boolean collectAndCommitIfNecessary(Collection<TopicPartition> partition
35963597
return false;
35973598
}
35983599
}
3599-
if (offsetsToCommit.size() > 0) {
3600+
if (!offsetsToCommit.isEmpty()) {
36003601
commitCurrentOffsets(offsetsToCommit);
36013602
}
36023603
return true;

0 commit comments

Comments
 (0)