Add Int.from_digits as inverse of Int#digits

[andrykonchin]

Changes:

• added Int.from_digits that returns a number from digits

Example:

Int32.from_digits([5, 4, 3, 2, 1])  # => 12345

Closes #14954

[HertzDevil]

I think these should not be hardcoded to Int32

[straight-shoota]

Yes, I think this method should be defined on the concrete types (Int32, UInt8 etc.). Then self defines the return type.

[andrykonchin]

Is it OK to use macros in this case?

{% for type in %w(Int8 Int16 Int32 Int64 Int128 UInt8 UInt16 UInt32 UInt64 UInt128) %}
  def {{type.id}}.from_digits(digits : Enumerable(Int), base : Int = 10) : self
    if base < 2
      raise ArgumentError.new("Invalid base #{base}")
    end

    num = 0
    multiplier = 1

    # ...

    num
  end
{% end %}

[straight-shoota]

It might be okay to define it on Int. It will only work on its concrete subclasses though.
Int.from_io works that way as well. Calling it directly doesn't compile, but Int32.from_io etc. work fine.

Obviously, it's confusing to have the method defined on Int when it doesn't work there. So maybe the macro solution is better.

[straight-shoota]

We could use a macro inherited hook instead of declaring on each child type directly. But for some reason, that doesn't seem to work on Int. Maybe because the inheritance is defined in the compiler? 🤔

struct Int
  macro inherited
    def self.from_digits(digits : Enumerable(Int), base : Int = 10) : self
      # ...
      ZERO    
    end
  end
end

Int32.from_digits([1, 2, 3, 4]) # Error: undefined method 'from_digits' for Int32.class

[andrykonchin]

Done

[HertzDevil]

The inheritance is done in the compiler, so no hooks are called. See #13836

[straight-shoota]

undigits is not a very descriptive name. Int.from_digits might be better. That was also the original suggestion in #14954.

[HertzDevil]

multiplier also needs to be in the target type, otherwise it would overflow for something like 10_000_000_000

[andrykonchin]

Done

[HertzDevil]

Multiplication should not be done for the last digit, otherwise this would overflow for numbers near type's maximum, e.g. Int32.from_digits([0xFF, 0xFF, 0xFF, 0x7F], base: 256) would set multiplier to 0x100000000 for the last loop. Ditto if multiplier is no longer hardcoded to Int32

[andrykonchin]

Nice catch. Done

[HertzDevil]

this i is unused; you only need either this or first_element to check for the last (or first) element

[andrykonchin]

Ah, right.

I tried first to skip the last element. But in that case we need to call Enumerable#size that means iterating the whole collection in general case. So the current approach with first_element seems to me slightly better from the performance point of view (in general case). Does it make sense?

[andrykonchin]

Done

[andrykonchin]

Is it OK to squash commits into a single one providing that the PR is approved?

[straight-shoota]

Please leave the commit history unchanged. That makes it easier to review.
We squash on merge.

See https://github.com/crystal-lang/crystal/blob/master/CONTRIBUTING.md#making-good-pull-requests

[Sija]

Suggestion: use already established convention (# => ... to return values, # raises ... for exceptions):

Suggested change

	# Int32.from_digits([1], base: -2) # => ArgumentError
	# Int32.from_digits([-1]) # => ArgumentError
	# Int32.from_digits([3], base: 2) # => ArgumentError
	# Int32.from_digits([1], base: -2) # raises ArgumentError
	# Int32.from_digits([-1]) # raises ArgumentError
	# Int32.from_digits([3], base: 2) # raises ArgumentError

[andrykonchin]

Done

[Sija]

Just a note: OP description is outdated.