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
