cpoerschke commented on code in PR #929:
URL: https://github.com/apache/solr/pull/929#discussion_r914001900
##########
solr/core/src/test/org/apache/solr/schema/TestPointFields.java:
##########
@@ -5289,9 +5289,9 @@ private void doTestSetQueries(String fieldName, String[]
values, boolean multiVa
SchemaField sf = h.getCore().getLatestSchema().getField(fieldName);
assertTrue(sf.getType() instanceof PointField);
- for (int i = 0; i < values.length; i++) {
+ for (String s : values) {
Review Comment:
minor: here `s` is chosen but below `value` is chosen. consistency can help
with code reading e.g. `value` in both places or `s` or `v` or so.
##########
solr/core/src/test/org/apache/solr/cloud/TestStressInPlaceUpdates.java:
##########
@@ -684,10 +684,9 @@ public SolrClient getClientForLeader() throws
KeeperException, InterruptedExcept
Slice shard1 =
clusterState.getCollection(DEFAULT_COLLECTION).getSlice(SHARD1);
leader = shard1.getLeader();
- for (int i = 0; i < clients.size(); i++) {
+ for (SolrClient client : clients) {
String leaderBaseUrl =
zkStateReader.getBaseUrlForNodeName(leader.getNodeName());
- if (((HttpSolrClient)
clients.get(i)).getBaseURL().startsWith(leaderBaseUrl))
- return clients.get(i);
+ if (((HttpSolrClient) client).getBaseURL().startsWith(leaderBaseUrl))
return client;
Review Comment:
```suggestion
if (((HttpSolrClient) client).getBaseURL().startsWith(leaderBaseUrl)) {
return client;
}
```
##########
solr/core/src/test/org/apache/solr/search/QueryEqualityTest.java:
##########
@@ -1351,13 +1351,13 @@ protected void assertQueryEquals(
SolrRequestInfo.clearRequestInfo();
}
- for (int i = 0; i < queries.length; i++) {
- QueryUtils.check(queries[i]);
+ for (Query value : queries) {
+ QueryUtils.check(value);
// yes starting j=0 is redundent, we're making sure every query
// is equal to itself, and that the quality checks work regardless
// of which caller/callee is used.
- for (int j = 0; j < queries.length; j++) {
- QueryUtils.checkEqual(queries[i], queries[j]);
+ for (Query query : queries) {
+ QueryUtils.checkEqual(value, query);
Review Comment:
How about naming to indicate that both are from `queries` ultimately? E.g.
```suggestion
QueryUtils.checkEqual(query1, query2);
```
##########
solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java:
##########
@@ -220,9 +220,8 @@ private void mapReplicasToClients() throws KeeperException,
InterruptedException
leader = shard1.getLeader();
String leaderBaseUrl =
zkStateReader.getBaseUrlForNodeName(leader.getNodeName());
- for (int i = 0; i < clients.size(); i++) {
- if (((HttpSolrClient)
clients.get(i)).getBaseURL().startsWith(leaderBaseUrl))
- LEADER = clients.get(i);
+ for (SolrClient solrClient : clients) {
+ if (((HttpSolrClient)
solrClient).getBaseURL().startsWith(leaderBaseUrl)) LEADER = solrClient;
Review Comment:
```suggestion
if (((HttpSolrClient)
solrClient).getBaseURL().startsWith(leaderBaseUrl)) {
LEADER = solrClient;
}
```
##########
solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java:
##########
@@ -231,9 +230,8 @@ private void mapReplicasToClients() throws KeeperException,
InterruptedException
continue;
}
String baseUrl = zkStateReader.getBaseUrlForNodeName(rep.getNodeName());
- for (int i = 0; i < clients.size(); i++) {
- if (((HttpSolrClient) clients.get(i)).getBaseURL().startsWith(baseUrl))
- NONLEADERS.add(clients.get(i));
+ for (SolrClient client : clients) {
+ if (((HttpSolrClient) client).getBaseURL().startsWith(baseUrl))
NONLEADERS.add(client);
Review Comment:
```suggestion
if (((HttpSolrClient) client).getBaseURL().startsWith(baseUrl)) {
NONLEADERS.add(client);
}
```
##########
solr/core/src/test/org/apache/solr/update/processor/SignatureUpdateProcessorFactoryTest.java:
##########
@@ -179,20 +179,20 @@ public void run() {
threads2[i].setName("testThread2-" + i);
}
- for (int i = 0; i < threads.length; i++) {
- threads[i].start();
+ for (Thread element : threads) {
+ element.start();
}
- for (int i = 0; i < threads2.length; i++) {
- threads2[i].start();
+ for (Thread item : threads2) {
+ item.start();
}
- for (int i = 0; i < threads.length; i++) {
- threads[i].join();
+ for (Thread value : threads) {
+ value.join();
}
Review Comment:
Curious about the `element/item/value` naming choices here, would `thread`
be clearer?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
