Show cost with 2 decimals #1677
Conversation
|
Thanks for the pull request. However, it needs to take all details into account. See my comments below. |
| // | ||
| // } | ||
| return getTreeTable().getDefaultRenderer(columnClass); | ||
| if (Double.class.equals(columnClass)){ |
There was a problem hiding this comment.
This way all columns with Double type will be formatted with two decimal digits. It is okay for cost (which is essentially money) but it may not be okay for other columns, including user-defined (imagine e.g. weight measured in tons).
I think that the best way to achieve the goal is to override method newTablecolumnext in GanttTreeTable and check if we are creating exactly COST column. We already use similar approach in ResourceTreeTable.
Also, the same should be done with the editor, no? Otherwise the user will see values with 2 decimal digits, but once he starts editing, he will see something different.
| }; | ||
|
|
||
| class DecimalRenderer extends DefaultTableCellRenderer { | ||
| private final DecimalFormat myFormatter = new DecimalFormat("#0.00"); |
There was a problem hiding this comment.
The format of representing currency values is locale-specific. You want to use NumberFormat::getCurrencyInstance and pass the currently selected locale. GanttLanguage is responsible for such things (and it stores the current locale)
| TableCellRenderer renderer = createCellRenderer(columnClass); | ||
| String columnName = getTreeTableModel().getColumnName(modelIndex).toLowerCase(); | ||
| TableCellRenderer renderer; | ||
| Boolean costColumn = false; |
There was a problem hiding this comment.
you don't need to use Boolean (capitalized) for a simple function-local boolean value. Atomic type is just fine.
|
|
||
| class DecimalRenderer extends DefaultTableCellRenderer { | ||
| // private final DecimalFormat myFormatter = new DecimalFormat("#0.00"); | ||
| private final DecimalFormat myFormatter = new DecimalFormat(NumberFormat.getNumberInstance(GanttLanguage.getInstance().getLocale()).toString()); |
There was a problem hiding this comment.
Pardon?
This fails because toString in format instances is not overridden and it is just a class name and memory address:
java.lang.IllegalArgumentException: Multiple decimal separators in pattern "java.text.DecimalFormat@674dc"
at java.base/java.text.DecimalFormat.applyPattern(DecimalFormat.java:3411)
at java.base/java.text.DecimalFormat.<init>(DecimalFormat.java:441)
at net.sourceforge.ganttproject.GPTreeTableBase$DecimalRenderer.<init>(GPTreeTableBase.java:97)
at net.sourceforge.ganttproject.GPTreeTableBase.newTableColumnExt(GPTreeTableBase.java:918)
Besides, you can just use NumberFormat directly without converting to Decimalformat.
This format instance will not update when user changes interface language in GanttProject settings. You need to listen to language change events
| private final DecimalFormat myFormatter = new DecimalFormat(NumberFormat.getNumberInstance(GanttLanguage.getInstance().getLocale()).toString()); | ||
|
|
||
| public Component getTableCellRendererComponent (JTable table, Object value, boolean isSelected, boolean hasFocus, int row, int column) { | ||
| value = myFormatter.format((Number)value); |
There was a problem hiding this comment.
Cast is not necessary. format argument is Object.
| String columnName = getTreeTableModel().getColumnName(modelIndex).toLowerCase(); | ||
| TableCellRenderer renderer; | ||
| Boolean costColumn = false; | ||
| if (Double.class.equals(columnClass) && columnName.equals("cost")) { |
There was a problem hiding this comment.
This will work only if your system locale is English. Should it be some other locale, column name will not be "cost".
You can find TaskDefaultColumn instance by model index: TaskDefaultColumn.values()[modelIndex]
| TableColumnExt result = new TableColumnExt(modelIndex); | ||
| Class<?> columnClass = getTreeTableModel().getColumnClass(modelIndex); | ||
| TableCellRenderer renderer = createCellRenderer(columnClass); | ||
| String columnName = getTreeTableModel().getColumnName(modelIndex).toLowerCase(); |
There was a problem hiding this comment.
This class is abstract and is shared between Gantt chart and Resource chart. Please move all checks which are related to tasks into GanttTreeTable
2 participants
Fixes #1659