Skip to content

Conversation

@akmhmgc
Copy link
Owner

@akmhmgc akmhmgc commented Oct 11, 2025

@akmhmgc akmhmgc added the ruby label Oct 11, 2025
end

max_value = 2 ** 31 - 1
min_value = (-1) * 2 ** 31
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ある種の定数なので、大文字で定義してもいいかもしれないと思いました。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

コメントありがとうございます!
Rubyはメソッドの外側でしか定数を定義できないので変数にしています。
メソッドの外に出してしまうと定数としては定義できますがスコープが広くなるので悩ましいですね…

index += 1
end
if str_without_left_space.start_with?("+")
index += 1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indexを進める代わりに、新しい文字列の変数を定義して、最後のwhile文をその文字列でfor_each_charを行う処理に変えてもよいと思いました。

digit = str_without_left_space[index].to_i

if result > (max_value - digit) / 10
return sign == 1 ? max_value : min_value

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"-2147483648" (min_value) が入力されたら Rounding の対象ではないが、この行で return されるように思いました。正しい値を返せていますが、少し引っかかりました。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ありがとうございます。
確かに改めて見るとMIN_VALUEなのにオーバーフロー扱いしているように見えるので気持ち悪いですね。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# @param {String} str
# @return {Integer}
def my_atoi(str)
    str_without_left_space = str.lstrip

    index = 0
    sign = 1
    if str_without_left_space.start_with?("-")
        sign = -1
        index += 1
    end
    if str_without_left_space.start_with?("+")
        index += 1
    end

    max_value = 2 ** 31 - 1
    min_value = (-1) * 2 ** 31
    limit = (sign == 1) ? max_value : 2 ** 31
    result = 0
    while index < str_without_left_space.size && str_without_left_space[index].between?("0", "9")
        digit = str_without_left_space[index].to_i
        
        if result > (limit - digit) / 10
            return sign == 1 ? max_value : min_value
        end

        result = result * 10 + digit
        index += 1
    end
    result * sign
end

こっちの方が良いかもしれません

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

絶対値の上限を正負で分けたということですね。自分だったら absolute_value_limit のようにします。
改善案がなく恐縮ですが、2 ** 31 とハードコーディングしている箇所が増えたのは保守性が落ちてしまっているかもしれないと思いました。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

absolute_value_limit = (sign == 1) ? max_value : (-1) * min_value

とするとかですかね。

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確かに Ruby だとそれで良いですね。

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants