Porting ho table model to kotlin #2177
Conversation
I experimented with this a bit, and one way of approaching this is to |
|
I'll have a look when I get a chance, there is a lot to unpack here :) |
There was a problem hiding this comment.
When we discussed introducing Kotlin, I remember we had proposed adding unit tests as part of the migration. Is that still something you're keen on doing? For Swing components this may be tricky, but for something like HOConfigurationIntParameter or HOConfigurationParameter this can have some value.
So, how do you find Kotlin? :)
| /** | ||
| * Position of the divider between fixed and scrollable tables | ||
| */ | ||
| private var dividerLocation: HOConfigurationIntParameter? = null |
There was a problem hiding this comment.
I am sure you've figured that out, but where possible we should use non-nullable types.
| */ | ||
| override fun setRowSorter(sorter: RowSorter<out TableModel?>) { | ||
| super.setRowSorter(sorter) | ||
| if (fixed != null) fixed!!.rowSorter = sorter |
There was a problem hiding this comment.
This is equivalent to
fixed?.rowSorter = sorter
| return try { | ||
| super.getColumn(identifier) | ||
| } catch (e: IllegalArgumentException) { | ||
| fixed!!.getColumn(identifier) |
There was a problem hiding this comment.
This can cause an NPE if fixed is null
| */ | ||
| fun getTableColumn(i: Int): TableColumn { | ||
| if (i < fixedColumnsCount) { | ||
| return fixed!!.columnModel.getColumn(i) |
There was a problem hiding this comment.
This can cause an NPE if fixed is null
| if (_displayedColumns == null) { | ||
| val displayedColumnsList = ArrayList<UserColumn>() | ||
| for (column in columns) { | ||
| if (column.isDisplay) { | ||
| displayedColumnsList.add(column) | ||
| } // column is displayed | ||
| } // for | ||
| _displayedColumns = displayedColumnsList.toTypedArray() | ||
| } | ||
| return _displayedColumns as Array<UserColumn> |
There was a problem hiding this comment.
I think this could be simplified to something like:
fun getDisplayedColumns(): Array<UserColumn> {
if (_displayedColumns == null) {
_displayedColumns = columns.filter { col -> col.isDisplay }.toTypedArray()
}
return _displayedColumns as Array<UserColumn>
}| * @return int | ||
| */ | ||
| override fun getRowCount(): Int { | ||
| return if ((m_clData != null)) m_clData!!.size else 0 |
There was a problem hiding this comment.
I think this can be shortened to return m_clData?.size ?: 0
| */ | ||
| fun initTable(table: JTable) { | ||
| this.table = table | ||
| if (table !is FixedColumnsTable) { |
There was a problem hiding this comment.
Well, didn't know that was possible, is! :D
| val sortKeys = ArrayList<RowSorter.SortKey>() | ||
| val sortColumns = Arrays.stream(this.columns).filter { i: UserColumn -> i.sortPriority != null }.sorted( | ||
| Comparator.comparingInt { obj: UserColumn -> obj.getSortPriority() }).toList() | ||
| if (!sortColumns.isEmpty()) { |
There was a problem hiding this comment.
This can become sortColumns.isNotEmpty()
| companion object { | ||
| @Serial | ||
| private val serialVersionUID = -207230110294902139L | ||
| } |
There was a problem hiding this comment.
I am still not sure whether there is any point to serialVersionUID. My understanding was that this was used in serialization/deserialization, and to ensure that the changes made to a serializable class were compatible so that older version of a serialized instance can still be deserialized (I remember a loooong time this mattered for example in RMI), but I don't believe we make sure changes are compatible, or if serialization is even used here.
| for (i in players!!.indices) { | ||
| val currentPlayer = players!![i] | ||
| for (j in tmpDisplayedColumns.indices) { | ||
| if (tmpDisplayedColumns[j].id == UserColumnFactory.NAME || tmpDisplayedColumns[j].id == UserColumnFactory.LINEUP || tmpDisplayedColumns[j].id == UserColumnFactory.BEST_POSITION || tmpDisplayedColumns[j].id == UserColumnFactory.SCHUM_RANK_BENCHMARK || tmpDisplayedColumns[j].id == UserColumnFactory.GROUP) { |
There was a problem hiding this comment.
Would you mind breaking up into multiple lines for readability?
changes proposed in this pull request:
My first steps in kotlin programming.
I'm really excited if this can lead to us being able to support Android in the near future (HO10 or HO11 !?).
I don't think it's so great that we lose the history of the changes on this tour.
src/main/resources/release_notes.md...