Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exceptionless string to number conversions #19

Closed
ilya-g opened this issue Jun 1, 2016 · 11 comments
Closed

Exceptionless string to number conversions #19

ilya-g opened this issue Jun 1, 2016 · 11 comments
Assignees
Labels

Comments

@ilya-g
Copy link
Member

ilya-g commented Jun 1, 2016

Discussions about the exceptionless string to number conversions will be held here.

@ilya-g ilya-g added the stdlib label Jun 1, 2016
@sksamuel
Copy link

Have you considered returning some "error" state rather than null. Something that could indicate why the string was not convertible (eg, "found 'a' expected 0-9). For this you would need to add some kind of Try object to the stdlib (which is a good thing too imo). But maybe that's getting a bit too Scala-like for some people.

Eg,

val result1 : Try[Int] = "hello".tryInt // Failure(ParseException("Found 'h' expected [0-9.]"))
val result2 : Try[Int] = "12.34".tryInt // Success(12.34)

@dnpetrov
Copy link
Contributor

For a solution focused on performance, you can consider something like

inline fun String.tryIntOrElse(default: String.() -> Int): Int // or maybe (String) -> Int

that would avoid allocating any objects for result, and could be used like

val example1 = "42".tryIntOrElse { 0 }
val example2 = "42".tryIntOrElse { throw AssertionError("$this is not an Int") }

Naming scheme is similar to getOrElse.

@ilya-g
Copy link
Member Author

ilya-g commented Jun 15, 2016

@dnpetrov I thought about something like inline fun toIntOrElse { }. Unfortunately, String->Number conversion isn't trivial, and it would require too much code to be inlined.

@voddan
Copy link
Contributor

voddan commented Jul 2, 2016

What''s the status on this issue? I don't see a set target version in https://youtrack.jetbrains.com/issue/KT-7930, so I assume the discussion is in the process.

@ilya-g
Copy link
Member Author

ilya-g commented Jul 4, 2016

@voddan We had an initial discussion about this proposal today, and found it appealing to adopt, however, we hadn't discussed the current issues in depth.

Btw, what do you think about toIntOrElse proposed by @dnpetrov?

@voddan
Copy link
Contributor

voddan commented Jul 4, 2016

A major problem I see with toIntOrElse is that if it returns Int it can't emulate toIntOrNull, which is a common use case, but if it returns Int?, we get the boxing and the performance is the same as "".toIntOrNull() ?: default

On the other hand, toIntOrNull is flexible enough to provide any semantics needed.

@voddan
Copy link
Contributor

voddan commented Jul 4, 2016

From my point of view, in practice there are two major use cases:

  1. Parsing a huge stream of preformatted data: use toInt since performance matters and the format is guarantied by the data source (for example, file format)
  2. Parsing a user's input: use toIntOrNull since the format is not guarantied. This is way faster than catching exceptions, and we lose exactly one boxing to toIntOrElse

IMHO, toIntOrElse is simply not needed, provided we have toIntOrNull. The reversed is not true

@voddan
Copy link
Contributor

voddan commented Jul 4, 2016

A note: I am not sure how it is possible to implement toIntOrElse {} with the following requirements:

  1. When parsed correctly, no boxing is used
  2. When parsed correctly, no overhead is created on default lambda
  3. The parsing algorithm is not inlined (it's fairly big)

@ilya-g
Copy link
Member Author

ilya-g commented Jul 4, 2016

A note: I am not sure how it is possible to implement toIntOrElse {} with the following requirements

These are exactly my concerns about toIntOrElse.

We have found another option to provide 'boxingless' conversion: String.toIntOrDefault(default: Int): Int.

Often, one might have sensible default value from the same domain: toIntOrDefault(0) for example, or the range of valid values doesn't cover the entire domain of Int, so a sentinel value could be found outside of that range to indicate parsing error: toIntOrDefault(Int.MIN_VALUE).

These functions might be provided in addition to toIntOrNull rather than instead of. It should be noted that it would require to duplicate parsing code one more time to return a default value instead of null.

@voddan
Copy link
Contributor

voddan commented Jul 4, 2016

@ilya-g completely agree on the above

I don't see the benefits of introducing 8 functions for avoiding one boxing. According to my theory #19 (comment) use cases that would significantly benefit from those do no exist

@ilya-g
Copy link
Member Author

ilya-g commented Jul 14, 2016

I'll remove a link to https://youtrack.jetbrains.com/issue/KT-9374 as it doesn't relate to this proposal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants