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

[RFC] Add Int#digits method #8554

Closed
asterite opened this issue Dec 4, 2019 · 7 comments
Closed

[RFC] Add Int#digits method #8554

asterite opened this issue Dec 4, 2019 · 7 comments

Comments

@asterite
Copy link
Member

asterite commented Dec 4, 2019

The idea is to have a digits method similar to what Ruby has:

123.digits # => [3, 2, 1]

Why?

The first question that we take into account when considering adding a new method to the standard library is: what's the use case?

Well, my use case, and I guess for others too, is using this method whenever it's needed. In my case it's for today's Advent of Code (day 4).

But wait! That's not a "real world" use case! Or is it...? I think we are solving these fun challenges in the real world, and one of Crystal's goals is to have fun, and it's definitely more fun to have this method than to not have it. Also, digits is much faster than doing number.to_s.chars.map(&.to_i): there's no need to create an intermediate string nor intermediate arrays.

Should 123.digits be [3, 2, 1] or [1, 2, 3]?

Excellent question!

Ruby returns [3, 2, 1] which might not be intuitive but it makes it so that the first digit is 3, the second one is 2, etc. I guess if we look at it like decimal places, in the first place it's the 0-10 range, the second place is the 10-100 range, etc. So it's probably fine. And one can always do 123.digits.reverse! if you want it the other way around, and it is what the method would need to do anyway because of how the array is built (diving and modding by 10).

I also think the return type should always be Array(Int32) because Int32 is the most common type to use, and digits can only be from 0 to 9.

There's already #5598 but we could probably start a new PR from scratch.

Thoughts?

(Edit: that's why I also think we should eventually add String#center and String#swapcase: well, the first one is very useful in real world scenarios... the second one probably not, but it's really easy to implement and it's nice when you find it there and it can actually help you solve a problem, be it a challenge or anything else)

@yxhuvud
Copy link
Contributor

yxhuvud commented Dec 4, 2019

To avoid creating too much overhead I ended up doing to_s to an IO, which I then picked out the slice which I iterated over. This is probably a cleaner approach.

Not that I can say I have needed digits outside AoC.

@jwoertink
Copy link
Contributor

one of Crystal's goals is to have fun, and it's definitely more fun to have this method than to not have it

I'm sold

@jkthorne
Copy link
Contributor

jkthorne commented Dec 4, 2019

This would be awesome for 128int support

@Blacksmoke16
Copy link
Member

Blacksmoke16 commented Dec 4, 2019

Is there any reasoning as to not include this in Number vs Int? The main issue would be how to handle the decimal I guess.

@straight-shoota
Copy link
Member

The main issue would be how to handle the decimal I guess.

Exactly. This wouldn't fit for floating point numbers. BigDecimal could work but then it's also the question how to separate the decimal part. Unless there's a great idea about this, I'd not bother about that.

@bew
Copy link
Contributor

bew commented Mar 12, 2020

Random idea:

Name the method digits_by_power(base = 10) to make it clear that the result are in the order of powers (base^0, base^1, base^2, ...), with a configurable base. (..power ? ..powers ?)

And also add an iterator each_digits_by_power(base = 10) to avoid allocating an array if you just want the first/last digit.

@asterite
Copy link
Member Author

asterite commented Jun 1, 2020

Implemented in #9383

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

No branches or pull requests

8 participants