ferenc-csaky commented on code in PR #27: URL: https://github.com/apache/flink-connector-hbase/pull/27#discussion_r1356575876
########## flink-connector-hbase-2.2/src/test/java/org/apache/flink/connector/hbase2/source/HBaseRowDataAsyncLookupFunctionTest.java: ########## @@ -74,8 +72,8 @@ public void testEval() throws Exception { assertTrue(result.size() < rowkeys.length); latch.await(); lookupFunction.close(); - List<String> sortResult = - Lists.newArrayList(result).stream().sorted().collect(Collectors.toList()); + List<String> sortResult = new ArrayList<>(result); + Collections.sort(sortResult); Review Comment: The `result` is not used at all after line 72, so IMO simply calling `Collections.sort(result)` and assert `result` against `expected` would be enough here I think. ########## flink-connector-hbase-2.2/src/test/java/org/apache/flink/connector/hbase2/HBaseConnectorITCase.java: ########## @@ -469,11 +470,13 @@ private void verifyHBaseLookupJoin(boolean async) { + TEST_TABLE_1 + " FOR SYSTEM_TIME AS OF src.proc as h ON src.a = h.rowkey"; Iterator<Row> collected = tEnv.executeSql(dimJoinQuery).collect(); - List<String> result = - Lists.newArrayList(collected).stream() - .map(Row::toString) - .sorted() - .collect(Collectors.toList()); + List<String> result = new ArrayList<>(); + for (Iterator<Row> it = collected; it.hasNext(); ) { + Row row = it.next(); + result.add(row.toString()); + } + Collections.sort(result); + result = new ArrayList<>(result); Review Comment: Same as my other comment. ########## flink-connector-hbase-2.2/src/test/java/org/apache/flink/connector/hbase2/HBaseConnectorITCase.java: ########## @@ -384,11 +383,13 @@ public void testTableSourceSinkWithDDL() throws Exception { TableResult tableResult3 = batchEnv.executeSql(query); - List<String> result = - Lists.newArrayList(tableResult3.collect()).stream() - .map(Row::toString) - .sorted() - .collect(Collectors.toList()); + List<String> result = new ArrayList<>(); + for (CloseableIterator<Row> it = tableResult3.collect(); it.hasNext(); ) { + Row row = it.next(); + result.add(row.toString()); + } + Collections.sort(result); + result = new ArrayList<>(result); Review Comment: I agree with @snuyanzin about the second deep copy. Furthermore, the most clean solution IMO would be this: ```java List<String> result = new ArrayList<>(); tableResult3.collect().forEachRemaining(r -> result.add(r.toString())); ``` -- 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. To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org