General feedback on the API design

Hello, this isn't regarding one specific issue per se, but rather some general feedback regarding the design of the current API. If any of this comes off as aggressive/mean sounding, I apologize, my intention is solely for constructive criticism.

Most of my opinions will be based on the API design of officially supported format libraries developed by JetBrains themselves, which can be found here, and the Kotlin coding conventions provided by JetBrains, which can be found here. I'm not sure how much stuff you wanna change, but I figured it would be best to provide feedback while the library is still in early development, as a lot of these changes would break backwards compatibility.

There's a decent chunk of stuff that I want to provide feedback on, so I'm sorry if things read like a jumbled mess. I will try to section off the feedback to their own "sections" as best as I can.

---

If any of the suggestions here are something you like, I can make a pull requests with the fixes if desired. I would rather just explain my reasoning and thoughts before just making a pull requests with all fixes.

---

The Ktoml class

The class name

First point to address here is the name, if we look at naming rules, it states that Names of classes and objects start with an uppercase letter and use camel case, and with camel case, each new word should be capitalized, and for acronyms, each letter representing the word should normally be treated as a new word. meaning that if we follow these rules, the appropriate name for the class should be KToml rather than Ktoml as the K stands for Kotlin, and toml should be treated as one word. (By following the above rules it should technically be KTOML, but if we look at the officially supported formats like json, and other classes developed by JetBrains, they seem to follow the rules of Dart wherein an acronym that's 3 or more characters long should be treated as a word, so instead of URL it would be Url.)

However, if we look at essentially all other libraries, even those outside of the officially supported formats, like yamlkt and avro4k, they just use the format name as the class name, meaning that rather than Ktoml it would be Toml.

Personally I think the nicest looking option is to just follow the official libraries and name the class Toml, as there is no real point in denoting that it's specifically for Kotlin as far as I can see.

The general design of the config

I will be basing the following suggestion on the json library.

If we look at how the json library handles configuration, we can see that it's using a sealed class hierarchy to achieve this, which can be roughly laid out like this:

• The Json class is the parent, you can not create new instances of directly, it has the implementations for the format its extending already defined.
• The companion object Default of the Json class is the default implementation, which uses the default settings for serialization/deserialization.
• There exists a JsonImpl class which allows custom settings to be set, this is internal and never exposed to the end user.
• There exists a JsonBuilder class which allows the user to customize the settings, in conjunction with the top-level Json function this provides a nice Kotlin DSL for creating a Json format with custom settings.
• The top-level Json function acts as the constructor of the Json class, allowing the user to create instances with a custom configuration easily.

The benefit of this structure is that if I just want to use the default settings for the format, I can just write Json.encodeToString, or Json.Default.encodeToString if I want to be more explicit. And when I want to change the settings I can just go Json { // stuff }. It also allows the user to easily copy the settings from any already created Json instance, while keeping the settings of the instance immutable.

If we apply the same design layout to ktoml it would roughly look like this:

public sealed class Toml(
    override val serializersModule: SerializersModule,
    public val ignoreUnknownNames: Boolean,
) : StringFormat {
    public companion object Default : Toml(EmptySerializersModule, false)

    public final override fun <T> encodeToString(serializer: SerializationStrategy<T>, value: T): String = // default implementation

    public final override fun <T> decodeFromString(deserializer: DeserializationStrategy<T>, string: String): T = // default implementation
}

public fun Toml(from: Toml = Toml.Default, builderAction: TomlBuilder.() -> Unit): Toml {
    val builder = TomlBuilder(from).apply(builderAction)
    return TomlImpl(builder.serializersModule, builder.ignoreUnknownNames)
}

public class TomlBuilder internal constructor(toml: Toml) {
    public var ignoreUnknownNames: Boolean = toml.ignoreUnknownNames

    public var serializersModule: SerializersModule = toml.serializersModule
}

private class TomlImpl(module: SerializersModule, ignoreUnknownNames: Boolean) : Toml(module, ignoreUnknownNames)

The deserialize and serialize top-level functions

This is partly down to personal preference, but the absence of anything similar from most libraries should also be a tell-tale sign.

I do not think having these top-level functions actually add anything of value, I can see that the thought behind being that it might be easier to just call the top-level function rather than having to create a new Ktoml instance and call the relevant function. However, I can only see that this would bring readability issues and ambiguity going down the line.

Here are some of the issues I can see would pop up from these functions:

• The names of them are very ambiguous. Sure, it's obvious that they're deserializing/serializing something, but it's not obvious what format they're being converted to, deserializeToml would be better, but it still feels like a code smell due to the other reasons defined below.
• From my own experience, it's much better to store/cache a kotlinx.serialization format as a constant value somewhere, as essentially all implementations are immutable and do not modify anything within itself, they can be used from multiple threads, so thread safety is not a concern. Therefore, creating a new instance every time you just wanna write/read something is a code smell, and should generally be avoided. Due to how these functions work, they all create a new instance just for this purpose.
• Building on the first point, if I have multiple formats in one project, it's very ambiguous what format the function deserialize would actually deserialize into.
• Unless something has changed, using the implicit serializer(). function like what is done in these functions is way slower than explicitly passing in a serializer, as it requires reflection rather than just a direct function call. So encouraging the use of that function by making these functions so easily accessible is not good design imo.

The dependency on okio

I personally think dragging in a whole dependency just for inbuilt support for reading from a file is rather excessive, and I know a lot of other people also would like the dependency graph of the libraries they use to be as minimal as possible.

The inbuilt functions for reading from a file aren't that much of a time-saver either: Ktoml.decodeFromFile(Thing.serializer(), "/foo/bar.toml") vs Ktoml.decodeFromString(Thing.serializer(), Path("/foo/bar.toml").readText()) (The above example is of course if you're on the JVM, but it's still relevant due to the argument below.)

There's also the fact that okio is not the only mulitplatform kotlin library that supports files, and while kotlinx.io is currently postponed, it will still be developed at one point, and there will certainly be more multiplatform file libraries developed. If this library then forces a dependency on okio this could be annoying for users who would rather use another library.

Therefore I think it would be better to not have explicit support for a specific file library, and rather just leave that up to the user. (Just quickly reading text from a file is more verbose in okio than in the Java path api with Kotlin extensions, but regardless, I don't think the minimal amount of boilerplate saved is worth explicitly forcing this library onto the user.)

---

These suggestions are mainly only for the public facing API, as I haven't looked too deeply into the more internal API.

I hope no offense was taken from this, this is only meant as constructive criticism for a library I'm looking forward to use once it gets more stable.