You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "matthiasblaesing (via GitHub)" <gi...@apache.org> on 2023/03/25 10:45:28 UTC
[GitHub] [netbeans] matthiasblaesing commented on a diff in pull request #5714: Cleanup the use of Double primitive wrapper.
matthiasblaesing commented on code in PR #5714:
URL: https://github.com/apache/netbeans/pull/5714#discussion_r1148343169
##########
ide/bugtracking.commons/src/org/netbeans/modules/bugtracking/issuetable/NodeTableModel.java:
##########
@@ -175,10 +175,10 @@ public void setProperties(ColumnDescriptor[] props) {
Object o = props[i].getValue(ATTR_ORDER_NUMBER);
- if (o instanceof Integer) {
- sort.put(new Double(((Integer) o).doubleValue()), Integer.valueOf(ia));
+ if ((o != null) && o instanceof Integer) {
+ sort.put(Double.valueOf(((Integer) o).doubleValue()), ia);
Review Comment:
I would drop the `Double.valueOf`, the dupplicate type conversion (Integer -> double -> Double) looks stange. I would assume, that the VM autoboxes here and I think
```java
sort.put(((Integer) o).doubleValue(), ia);
```
is more readable.
##########
platform/openide.explorer/src/org/openide/explorer/view/NodeTableModel.java:
##########
@@ -218,9 +218,9 @@ private void computeVisiblePorperties(int visCount) {
int vi = allPropertyColumns[i].getVisibleIndex();
if (vi == -1) {
- sort.put(new Double(i - 0.1), Integer.valueOf(i));
+ sort.put(Double.valueOf(i - 0.1), Integer.valueOf(i));
} else {
- sort.put(new Double(vi), Integer.valueOf(i));
+ sort.put(Double.valueOf(vi), Integer.valueOf(i));
Review Comment:
I would assume, that autoboxing should do the right thing. In line 221 both `#valueOf` calls can be dropped and in line 223 the second `#valueOf` can be dropped and the first can be replaced with a direct cast.
##########
ide/bugtracking.commons/src/org/netbeans/modules/bugtracking/issuetable/NodeTableModel.java:
##########
@@ -175,10 +175,10 @@ public void setProperties(ColumnDescriptor[] props) {
Object o = props[i].getValue(ATTR_ORDER_NUMBER);
- if (o instanceof Integer) {
- sort.put(new Double(((Integer) o).doubleValue()), Integer.valueOf(ia));
+ if ((o != null) && o instanceof Integer) {
Review Comment:
This is not necessary `null` is never in instance of `Integer`. If `o instanceof Integer` yields `true`, it is implied, that `o != null`.
##########
profiler/profiler.heapwalker/src/org/netbeans/modules/profiler/heapwalk/ClassesListController.java:
##########
@@ -164,7 +164,7 @@ public Object[][] getData(String[] filterStrings, int filterType, boolean showZe
else data[i][4] = (retainedSizeByClass > 0 ? "+" : "") + numberFormat.format(retainedSizeByClass); // NOI18N
}
} else {
- data[i][1] = new Double((double) instancesCount /
+ data[i][1] = Double.valueOf((double) instancesCount /
Review Comment:
This `Double#valueOf` wrap can be dropped. The type of the inner expression is `double`, autoboxing should do the rest.
##########
ide/db/src/org/netbeans/modules/db/explorer/dlg/CreateTableDialog.java:
##########
@@ -471,7 +471,7 @@ public DataTable(TableModel model) {
Map cmap = ColumnItem.getColumnProperty(i);
col.setIdentifier(cmap.get("name")); //NOI18N
columnName = NbBundle.getMessage (CreateTableDialog.class, "CreateTable_" + i); //NOI18N
- columnWidth = (new Double(getFontMetrics(getFont()).getStringBounds(columnName, getGraphics()).getWidth())).intValue() + 20;
+ columnWidth = Double.valueOf(getFontMetrics(getFont()).getStringBounds(columnName, getGraphics()).getWidth()).intValue() + 20;
Review Comment:
Would it make sense to convert the `Double.valueOf(/*double yielding expression*/).intValue()` to `(int) /*double yielding expression*/`?
The above would then become:
```suggestion
columnWidth = ((int) getFontMetrics(getFont()).getStringBounds(columnName, getGraphics()).getWidth()) + 20;
```
##########
ide/editor.completion/src/org/netbeans/modules/editor/completion/PatchedHtmlRenderer.java:
##########
@@ -176,7 +176,7 @@ private static double _renderPlainString(
}
double chWidth = wid / chars.length;
- int estCharsToPaint = new Double(w / chWidth).intValue();
+ int estCharsToPaint = Double.valueOf(w / chWidth).intValue();
Review Comment:
Continued from `CreateTableDialog.java`, line 474:
```suggestion
int estCharsToPaint = (int) (w / chWidth);
```
This pattern is found multiple times in this PR.
##########
javafx/javafx2.samples/SwingInterop/src/swinginterop/SampleTableModel.java:
##########
@@ -37,9 +37,9 @@ public class SampleTableModel extends AbstractTableModel {
private final String[] names = {"2007", "2008", "2009"};
private Object[][] data = {
- {new Double(567), new Double(956), new Double(1154)},
- {new Double(1292), new Double(1665), new Double(1927)},
- {new Double(1292), new Double(2559), new Double(2774)}
+ {Double.valueOf(567D), Double.valueOf(956D), Double.valueOf(1154D)},
+ {Double.valueOf(1292D), Double.valueOf(1665D), Double.valueOf(1927D)},
+ {Double.valueOf(1292D), Double.valueOf(2559D), Double.valueOf(2774D)}
Review Comment:
I would expect, that the again autoboxing should kick in, so dropping the `Double#valueOf` calls should be of, if the `D` suffix is retained.
##########
platform/openide.explorer/src/org/openide/explorer/view/NodeTableModel.java:
##########
@@ -162,10 +162,10 @@ public void setProperties(Property[] props) {
Object o = props[i].getValue(ATTR_ORDER_NUMBER);
- if (o instanceof Integer) {
- sort.put(new Double(((Integer) o).doubleValue()), Integer.valueOf(ia));
+ if ((o != null) && o instanceof Integer) {
Review Comment:
The `null` check is unnessary with an `instanceof` check (see discussion above)
The structure can be simplified as in `ide/bugtracking.commons/src/org/netbeans/modules/bugtracking/issuetable/NodeTableModel.java`
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists