cpoerschke commented on a change in pull request #169:
URL: https://github.com/apache/solr/pull/169#discussion_r650383669
##########
File path:
solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/DbscanEvaluator.java
##########
@@ -90,8 +88,7 @@ public Object doWork(Object... values) throws IOException {
}
}
- @SuppressWarnings({"rawtypes"})
- Map fields = new HashMap();
+ Map<String, Object> fields = new HashMap<>();
Review comment:
@cpoerschke note to self: return later to consider this more, from a
quick look `s/String/Object` might be required given the `ClusterTuple` and
`Tuple` constructor signatures
```suggestion
Map<Object, Object> fields = new HashMap<>();
```
##########
File path:
solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/CloudSolrStream.java
##########
@@ -284,12 +284,10 @@ public void open() throws IOException {
return solrStreams;
}
- @SuppressWarnings({"unchecked"})
private StreamComparator parseComp(String sort, String fl) throws
IOException {
String[] fls = fl.split(",");
- @SuppressWarnings({"rawtypes"})
- HashSet fieldSet = new HashSet();
+ HashSet<String> fieldSet = new HashSet<>();
Review comment:
observation which the UI won't let me add at line 309/311 below:
`if(!fieldSet.contains(spec[0])) {` is what the does but the preceding `String
fieldName = spec[0].trim();` makes me wonder if it should be
`if(!fieldSet.contains(fieldName)) {` actually instead. technically unrelated
to the changes in this pull request of course but just noticed whilst
confirming that `HashSet<String>` here is correct.
##########
File path:
solr/contrib/prometheus-exporter/src/java/org/apache/solr/prometheus/collector/SchedulerMetricsCollector.java
##########
@@ -77,9 +77,8 @@ public void start() {
scheduler.scheduleWithFixedDelay(this::collectMetrics, 0, duration,
timeUnit);
}
- private@SuppressWarnings({"try"})
Review comment:
observation: `private@SuppressWarnings` is surprising here, if somehow
that resulted in the `private` not taking effect then the addition of `private`
would constitute as non-backcompat change. based on
https://solr.apache.org/docs/8_8_2/solr-prometheus-exporter/org/apache/solr/prometheus/collector/SchedulerMetricsCollector.html
however the `private` is taking effect i.e. no backcompat concerns here then
##########
File path:
solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/GaussFitEvaluator.java
##########
@@ -81,7 +80,7 @@ public Object doWork(Object... objects) throws IOException{
double[] coef = curveFitter.fit(pointList);
Gaussian gaussian = new Gaussian(coef[0], coef[1], coef[2]);
- List list = new ArrayList();
+ List<Double> list = new ArrayList<>();
Review comment:
suggestion: based on `VectorFunction` constructor variants
```suggestion
List<Number> list = new ArrayList<>();
```
##########
File path:
solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/PolyFitDerivativeEvaluator.java
##########
@@ -93,8 +92,7 @@ public Object doWork(Object... objects) throws IOException{
PolynomialFunction pf = new PolynomialFunction(coef);
UnivariateFunction univariateFunction = pf.derivative();
- @SuppressWarnings({"rawtypes"})
- List list = new ArrayList();
+ List<Double> list = new ArrayList<>();
Review comment:
subjective/optional:
```suggestion
List<Number> list = new ArrayList<>();
```
##########
File path:
solr/solrj/src/java/org/apache/solr/client/solrj/beans/DocumentObjectBinder.java
##########
@@ -408,28 +406,28 @@ private Object getFieldValue(SolrDocument solrDocument) {
}
}
- @SuppressWarnings({"unchecked", "rawtypes"})
<T> void inject(T obj, SolrDocument sdoc) {
Object val = getFieldValue(sdoc);
if(val == null) {
return;
}
if (isArray && !isContainedInMap) {
- List list;
+ List<?> list;
if (val.getClass().isArray()) {
set(obj, val);
return;
} else if (val instanceof List) {
- list = (List) val;
+ list = (List<?>) val;
} else {
- list = new ArrayList();
- list.add(val);
+ List<Object> temp = new ArrayList<>();
+ temp.add(val);
+ list = temp;
Review comment:
observation: interesting that this pattern is needed.
##########
File path:
solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/FetchStream.java
##########
@@ -204,9 +205,8 @@ public void setStreamContext(StreamContext streamContext) {
return l;
}
- @SuppressWarnings({"unchecked", "rawtypes"})
public void open() throws IOException {
- tuples = new ArrayList().iterator();
+ tuples = Collections.emptyIterator();
Review comment:
subjective:
```suggestion
tuples = Collections.emptyListIterator();
```
##########
File path:
solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/SearchStream.java
##########
@@ -227,13 +227,12 @@ public StreamComparator getStreamSort() {
return comp;
}
- @SuppressWarnings({"unchecked", "rawtypes"})
private StreamComparator parseComp(String sort, String fl) throws
IOException {
- HashSet fieldSet = null;
+ HashSet<String> fieldSet = null;
Review comment:
observation: as above w.r.t. `!fieldSet.contains(spec[0])` vs.
`!fieldSet.contains(fieldName)`
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
