simplified sorting by constructor

Kotlin · Apr 29, 2024 · fca4b8f · fca4b8f
1 parent 07d933b
commit fca4b8f
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 90 deletions.
diff --git a/...generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/toDataFrame.kt b/...generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/toDataFrame.kt
@@ -21,7 +21,7 @@ import org.jetbrains.kotlinx.dataframe.impl.getListType
 import org.jetbrains.kotlinx.dataframe.impl.isArray
 import org.jetbrains.kotlinx.dataframe.impl.isGetterLike
 import org.jetbrains.kotlinx.dataframe.impl.projectUpTo
-import org.jetbrains.kotlinx.dataframe.impl.schema.sortWithConstructors
+import org.jetbrains.kotlinx.dataframe.impl.schema.sortWithConstructor
 import java.lang.reflect.InvocationTargetException
 import java.lang.reflect.Method
 import java.time.temporal.Temporal
@@ -178,8 +178,8 @@ internal fun convertToDataFrame(
                 .filter { it.visibility == KVisibility.PUBLIC && it.isGetterLike() }
         }
 
-        // sort properties by order in constructors
-        .sortWithConstructors(clazz)
+        // sort properties by order in constructor
+        .sortWithConstructor(clazz)
 
     val columns = properties.mapNotNull {
         val property = it

diff --git a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/schema/Utils.kt b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/schema/Utils.kt
@@ -144,53 +144,38 @@ internal fun DataFrameSchema.createEmptyDataFrame(numberOfRows: Int): AnyFrame =
 internal fun createEmptyDataFrameOf(clazz: KClass<*>): AnyFrame =
     MarkersExtractor.get(clazz).schema.createEmptyDataFrame()
 
+/**
+ * Returns a map of property names to their order in the primary/single constructor, if it exists,
+ * `null` otherwise.
+ */
 internal fun getPropertyOrderFromPrimaryConstructor(clazz: KClass<*>): Map<String, Int>? =
-    clazz.primaryConstructor
+    (clazz.primaryConstructor ?: clazz.constructors.singleOrNull())
         ?.parameters
         ?.mapNotNull { it.name }
         ?.mapIndexed { i, v -> v to i }
         ?.toMap()
 
-internal fun getPropertyOrderFromAllConstructors(clazz: KClass<*>): List<Map<String, Int>> =
-    clazz.constructors
-        .map { constructor ->
-            constructor.parameters
-                .mapNotNull { it.name }
-                .mapIndexed { i, v -> v to i }
-                .toMap()
-        }
-
 /**
- * Sorts [this] according to the order of them in the constructors of [klass].
- * It prefers the primary constructor if it exists, else it falls back to the other constructors to do the sorting.
- * Finally, it falls back to lexicographical sorting if a property does not exist in any constructor.
+ * Sorts [this] according to the order of their [columnName] in the primary/single constructor of [klass]
+ * if it exists, else, it falls back to lexicographical sorting.
  */
-internal fun <T> Iterable<KCallable<T>>.sortWithConstructors(klass: KClass<*>): List<KCallable<T>> {
+internal fun <T> Iterable<KCallable<T>>.sortWithConstructor(klass: KClass<*>): List<KCallable<T>> {
     require(all { it.isGetterLike() })
     val primaryConstructorOrder = getPropertyOrderFromPrimaryConstructor(klass)
-    val allConstructorsOrders = getPropertyOrderFromAllConstructors(klass)
 
-    // starting off lexicographically, sort properties according to the order of all constructors
-    val allConstructorsSortedProperties = allConstructorsOrders
-        .fold(this.sortedBy { it.columnName }) { props, constructorOrder ->
-            props
-                .withIndex()
-                .sortedBy { (i, it) -> constructorOrder[it.columnName] ?: i }
-                .map { it.value }
-        }.toList()
+    // starting off lexicographically
+    val lexicographicalColumns = sortedBy { it.columnName }
 
+    // if no primary constructor, return lexicographical order
     if (primaryConstructorOrder == null) {
-        return allConstructorsSortedProperties
+        return lexicographicalColumns
     }
 
-    // prefer to sort properties according to the order in the primary constructor if it exists.
-    // if a property does not exist in the primary constructor, fall back to the other order
-
+    // else sort the ones in the primary constructor first according to the order in there
+    // leave the rest at the end in lexicographical order
     val (propsInConstructor, propsNotInConstructor) =
-        this.partition { it.columnName in primaryConstructorOrder.keys }
-
-    val allConstructorsSortedPropertyNames = allConstructorsSortedProperties.map { it.columnName }
+        lexicographicalColumns.partition { it.columnName in primaryConstructorOrder.keys }
 
-    return propsInConstructor.sortedBy { primaryConstructorOrder[it.columnName] } +
-        propsNotInConstructor.sortedBy { allConstructorsSortedPropertyNames.indexOf(it.columnName) }
+    return propsInConstructor
+        .sortedBy { primaryConstructorOrder[it.columnName] } + propsNotInConstructor
 }
diff --git a/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt
@@ -319,21 +319,11 @@ class CreateDataFrameTests {
         private var a: Int = 0
         private var b: String = ""
 
-        constructor()
-
         constructor(b: String, a: Int) {
             this.a = a
             this.b = b
         }
 
-        constructor(b: String) {
-            this.b = b
-        }
-
-        constructor(a: Int) {
-            this.a = a
-        }
-
         fun getA(): Int = a
         fun setA(a: Int) {
             this.a = a

diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/toDataFrame.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/toDataFrame.kt
@@ -21,7 +21,7 @@ import org.jetbrains.kotlinx.dataframe.impl.getListType
 import org.jetbrains.kotlinx.dataframe.impl.isArray
 import org.jetbrains.kotlinx.dataframe.impl.isGetterLike
 import org.jetbrains.kotlinx.dataframe.impl.projectUpTo
-import org.jetbrains.kotlinx.dataframe.impl.schema.sortWithConstructors
+import org.jetbrains.kotlinx.dataframe.impl.schema.sortWithConstructor
 import java.lang.reflect.InvocationTargetException
 import java.lang.reflect.Method
 import java.time.temporal.Temporal
@@ -178,8 +178,8 @@ internal fun convertToDataFrame(
                 .filter { it.visibility == KVisibility.PUBLIC && it.isGetterLike() }
         }
 
-        // sort properties by order in constructors
-        .sortWithConstructors(clazz)
+        // sort properties by order in constructor
+        .sortWithConstructor(clazz)
 
     val columns = properties.mapNotNull {
         val property = it

diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/schema/Utils.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/schema/Utils.kt
@@ -144,53 +144,38 @@ internal fun DataFrameSchema.createEmptyDataFrame(numberOfRows: Int): AnyFrame =
 internal fun createEmptyDataFrameOf(clazz: KClass<*>): AnyFrame =
     MarkersExtractor.get(clazz).schema.createEmptyDataFrame()
 
+/**
+ * Returns a map of property names to their order in the primary/single constructor, if it exists,
+ * `null` otherwise.
+ */
 internal fun getPropertyOrderFromPrimaryConstructor(clazz: KClass<*>): Map<String, Int>? =
-    clazz.primaryConstructor
+    (clazz.primaryConstructor ?: clazz.constructors.singleOrNull())
         ?.parameters
         ?.mapNotNull { it.name }
         ?.mapIndexed { i, v -> v to i }
         ?.toMap()
 
-internal fun getPropertyOrderFromAllConstructors(clazz: KClass<*>): List<Map<String, Int>> =
-    clazz.constructors
-        .map { constructor ->
-            constructor.parameters
-                .mapNotNull { it.name }
-                .mapIndexed { i, v -> v to i }
-                .toMap()
-        }
-
 /**
- * Sorts [this] according to the order of them in the constructors of [klass].
- * It prefers the primary constructor if it exists, else it falls back to the other constructors to do the sorting.
- * Finally, it falls back to lexicographical sorting if a property does not exist in any constructor.
+ * Sorts [this] according to the order of their [columnName] in the primary/single constructor of [klass]
+ * if it exists, else, it falls back to lexicographical sorting.
  */
-internal fun <T> Iterable<KCallable<T>>.sortWithConstructors(klass: KClass<*>): List<KCallable<T>> {
+internal fun <T> Iterable<KCallable<T>>.sortWithConstructor(klass: KClass<*>): List<KCallable<T>> {
     require(all { it.isGetterLike() })
     val primaryConstructorOrder = getPropertyOrderFromPrimaryConstructor(klass)
-    val allConstructorsOrders = getPropertyOrderFromAllConstructors(klass)
 
-    // starting off lexicographically, sort properties according to the order of all constructors
-    val allConstructorsSortedProperties = allConstructorsOrders
-        .fold(this.sortedBy { it.columnName }) { props, constructorOrder ->
-            props
-                .withIndex()
-                .sortedBy { (i, it) -> constructorOrder[it.columnName] ?: i }
-                .map { it.value }
-        }.toList()
+    // starting off lexicographically
+    val lexicographicalColumns = sortedBy { it.columnName }
 
+    // if no primary constructor, return lexicographical order
     if (primaryConstructorOrder == null) {
-        return allConstructorsSortedProperties
+        return lexicographicalColumns
     }
 
-    // prefer to sort properties according to the order in the primary constructor if it exists.
-    // if a property does not exist in the primary constructor, fall back to the other order
-
+    // else sort the ones in the primary constructor first according to the order in there
+    // leave the rest at the end in lexicographical order
     val (propsInConstructor, propsNotInConstructor) =
-        this.partition { it.columnName in primaryConstructorOrder.keys }
-
-    val allConstructorsSortedPropertyNames = allConstructorsSortedProperties.map { it.columnName }
+        lexicographicalColumns.partition { it.columnName in primaryConstructorOrder.keys }
 
-    return propsInConstructor.sortedBy { primaryConstructorOrder[it.columnName] } +
-        propsNotInConstructor.sortedBy { allConstructorsSortedPropertyNames.indexOf(it.columnName) }
+    return propsInConstructor
+        .sortedBy { primaryConstructorOrder[it.columnName] } + propsNotInConstructor
 }
diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt
@@ -319,21 +319,11 @@ class CreateDataFrameTests {
         private var a: Int = 0
         private var b: String = ""
 
-        constructor()
-
         constructor(b: String, a: Int) {
             this.a = a
             this.b = b
         }
 
-        constructor(b: String) {
-            this.b = b
-        }
-
-        constructor(a: Int) {
-            this.a = a
-        }
-
         fun getA(): Int = a
         fun setA(a: Int) {
             this.a = a