Public review of the Standard Library APIs [Closed]

Configure in that case should be an extension function and then the point is moot:

myObject.apply { configure() }

Technically speaking that’s an option, but I typically only use extension functions for things that I feel “belong” on the type. An “arbitrary” helper method doesn’t.

I’ve found that working with match results in Regex API is not very nice.

val regex = Regex("""(\d+)-(\d+)""")
val str = "123-456"
val match = regex.matchEntire(str)
if (match == null) error("must match")
val g1 = match.groups[1]!!.value

First problem is, I have to write !! after match.groups[1]. I understand that it’s theoretically possible to write regexp, which will return null group. That’s possible result from Java regex API. But in practice I’ve never saw such regex. In my experience, group is always not-null. That’s the first problem. Second problem is that almost always I need only string value from group. So that !!.value suffix is just boring syntax noise.

I think that MatchResult class should define

public operator fun get(index: Int): String = groups[index]?.value ?: ""

now I can write

val g1 = match[1]

it’s very simple and similar to JavaScript.

2 Likes

null groups are real:

  val r = Regex("(a+)|(b+)").matchEntire("bbb")
  if (r != null) {
    println(r.groups[1]) // null
    println(r.groups[2]) // bbb
  }

I agree that r.groups[2]!!.value doesn’t look great, but is there a better alternative?

I agree that they are real. But in my experience almost always they can’t be null. So I think that API usage should allow to “by-pass” null checking for those 90% patterns. Of course if one does need nullable groups, API is there, it’s not changing, actually. I’m just suggesting a shorter alternative: fun get(index: Int): String defined in MatchResult class. Something like shortcut for common use-case.

I see, makes sense.

File Readers/Writers

File.reader() should return an InputStreamReader rather than a FileReader. (FileReader just hardcodes default encoding and buffer size, and does not provide any additional methods.) It should allow to specify a Charset, which should default to UTF-8.
If there is a proven need to support FileReader as well, a separate File.fileReader() method should be introduced.

File.bufferedReader() should allow to specify a Charset, which should default to UTF-8.

File.writer() should return an OutputStreamWriter rather than a FileWriter. (FileWriter just hardcodes default encoding and buffer size, and does not provide any additional methods.) It should allow to specify a Charset, which should default to UTF-8.
If there is a proven need to support FileWriter as well, a separate File.fileWriter() method should be introduced.

File.bufferedWriter() should allow to specify a Charset, which should default to UTF-8.

File.printWriter() should allow to specify a Charset, which should default to UTF-8.

File extension method overloads that accept encoding as a String rather than a Charset seem redundant (use Charsets for common cases and Charset.forName (or your own abstraction) otherwise), and should be removed.

Asymmetry: File has a readLines extension method (which produces a List), whereas Reader has a useLines extension method (which produces a Sequence).

We’re going to change the behaviour of deleteRecursively and make it return false only in case of incomplete (partial) deletion, and true in all other cases, such as missing directory being deleted.

1 Like

We believe that instantiating a buffered char reader from file is more common task than buffered byte reader, so the former deserves to have a shortcut.

I tend to agree, we should make assertions API exhibit the same fail-fast behavior on both platforms.

This change would be rather disruptive at this moment, does it provide the value that justifies it?
Also, consider the foldIndexed/reduceIndexed PR 796, which we’re going to merge soon, does it feel natural to have index parameter after T in that case?

I believe the proposed order is more logical, and the current order will take people by surprise. I’ll leave it to you whether you consider this worth fixing at this point. All I can say is that I’d happily adapt my own code, and that IntelliJ would likely do this for me anyways.

For any method foo, the order used for coll.fooIndexed should be consistent with coll.withIndex().foo. In my proposal, the order of IndexedValue arguments would change, hence foldRightIndexed would use (R, Int, T) -> R.

PS: For the record, Scala only has withIndex (named zipWithIndex), to avoid a combinatorial explosion of methods (http://www.scala-lang.org/old/node/1569.html#comment-5189).

Function kotlin.system.measureTimeNano() should be named measureTimeNanos (or measureNanoTime).

File APIs

From what I can tell, Kotlin adopts the old Java way of reporting errors by returning null (e.g. in extension method File.listFiles) or false (e.g. in extension method File.deleteRecursively). This approach seems outdated, with newer File APIs such as those found in Apache Commons, Guava, and JDK 1.7+ generally favoring exceptions, and in some cases offering a “quiet” variant (which could perhaps also be a named parameter).

Implementation note: Not using JDK 1.7+ File APIs when available seems risky, and may degrade experience for users running on these platforms, compared to using JDK File APIs directly (in regard to performance, atomicity, correct symlink handling, etc.).

These overloads would be deprecated and removed soon.

forEachLine/forEachBlock would perform an action on element, same as forEach, and scope functions (let/run/with/apply etc) take a block and execute it.

We have plans after 1.0 to add some common types from java.lang and java.util to kotlin package as type aliases.

We cannot provide a method which returns a sequence of lines for File since it implies keeping stream open until the sequence is consumed and it’s unclear who should close the stream.
However, we could provide useLines for File and readLines for Reader.

I’ve also spotted this inconvenience when going through advent of code.
To mitigate this issue we’re going to provide a property MatchResult.groupValues. It would differ from groups in following:

  • returns a list of strings. This list is a wrapper around groups collection.
  • does not include zeroth group value corresponding to the entire match, so that groupValues[0] returns the value of the first group in regex.
  • if some group was not matched the corresponding value in groupValues would be an empty string ("").

Another solution that comes to mind: Do away with nullable groups, make MatchResult.value return empty string in case of a non-match (or, perhaps better, throw an exception), and add a boolean method MatchResult.isMatch.

So result.groups[3]!!.value == result.groupValues[2]? Sounds like asking for trouble to me (very similar looks/names, incompatible indices).

I would like an assertion function for equal doubles (the one with additional tolerance parameter) to be added to kotlin-test. I have to use JUnit’s Assert.assertEquals instead of Kotlin’s assertions for this. This introduces a bit of inconsistency into tests:

// Need to import org.junit.Assert for this
assertEquals(corner.angle, Math.PI/2, 1e-10)

Another assertion function I’ve missed is assertSame.

1 Like

The split method is not as handy as the Java counterpart, which accepts a plain string instead of a compiled Regex or Pattern. The existing split methods are especially confusing, because the Java counterpart has the same name and almost same signature.

I suggest to change the signature to fun split( regex: String, limit: Int = 0) or to add such a variant.

Java’s way of having some methods treat strings as plain strings, and other methods treat strings as regular expressions, is an anti-pattern. That’s probably why Kotlin uses distinct types. This clearly expresses the difference in the type system, avoiding bugs and puzzlers like "what’s the difference between String.replace(CharSequence, CharSequence) and String.replaceAll(String, String)", which many Java devs get wrong.