Skip to content

8. String to Integer (atoi) #57

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

fhiyo
Copy link
Owner

@fhiyo fhiyo commented Aug 24, 2024

- https://github.com/Mike0121/LeetCode/pull/23

digits部分をまとめてから一気に変換している。先に `s = s.lstrip()` するのもなるほど。
超巨大な数になる場合、最後までparseを止めないため不利になる実装。

Choose a reason for hiding this comment

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

もしこの方法で巨大数のコストを減らすとしたら、sを適当に短くする(パース対象を長さで切ってしまう)というのもあります。

  • lstrip()でstringが複製されるコストを問題としないのであれば、新しく短くしたsを作ってもよいと思います
  • この場合、符号の後の0を適宜読み飛ばす必要はあります

while i < len(s) and s[i] in Solution.DIGITS:
digit = ord(s[i]) - ord('0')
i += 1
abs_value *= 10

Choose a reason for hiding this comment

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

この10をlen(Solution.DIGITS)か、あるいはBASE_NUMBER = len(DIGITS)みたいなもので定義しておくと、'01234567890abcdef'や'01234567'への対応が一箇所で済みますね。
(ただ、このコードでそこまでやるか?は微妙なところですが)

Copy link
Owner Author

Choose a reason for hiding this comment

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

たしかに10の意味するところを定数として定義しておく方が変更に強そうですね。
一応 ord(s[i]) - ord('0') の処理がa-fでは破綻するのでそこも修正必要そうですかね。 int(s[i], BASE_NUMBER) が無難かな...

Choose a reason for hiding this comment

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

ほんとですね、a-fはダメですね、すみません、、

while is_digit(i):
digits.append(s[i])
i += 1
num = int(''.join(digits))

Choose a reason for hiding this comment

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

1文字の文字列の配列を作ってからjoin()するのはコストがかかりそうなので、この場合はindexを使って部分文字列を取るほうがよいですかね。あと別の方法としては正規表現で置換するなどもあります。

Copy link
Owner Author

Choose a reason for hiding this comment

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

正規表現、たしかにできますね。ありがとうございます。

class Solution:
    INT_MAX = (1 << 31) - 1
    INT_MIN = -(1 << 31)

    def myAtoi(self, s: str) -> int:
        m = re.match(r'\s*([+-]?[0-9]+)', s)
        if not m:
            return 0
        v = int(m[1])
        if v > Solution.INT_MAX:
            return Solution.INT_MAX
        if v < Solution.INT_MIN:
            return Solution.INT_MIN
        return v

こういう感じですかね

@Mike0121
Copy link

全体的に良いと思いました。3rdが一番読みやすく感じました。

s[index] == '6' or \
s[index] == '7' or \
s[index] == '8' or \
s[index] == '9'
Copy link

Choose a reason for hiding this comment

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

s[index] in "0123456789" という書き方があります。

Copy link

Choose a reason for hiding this comment

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

あ、下にありました。

integer *= 10
integer += int(s[i])
if integer >= Solution.INT_MAX + 1:
integer = Solution.INT_MAX + 1
Copy link

Choose a reason for hiding this comment

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

念の為、書きますが、ここで32ビットからは溢れてますね。
INT_MIN = -(INT_MAX + 1) なんですよね。

MAX_INT = (1 << 31) - 1
MIN_INT = -(1 << 31)
SIGNS = '+-'
DIGITS = '0123456789'
Copy link
Owner Author

Choose a reason for hiding this comment

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

メモ: 代わりにstring.digitsを使っても良さそう

while i < len(s) and s[i] in Solution.DIGITS:
digit = ord(s[i]) - ord('0')
i += 1
abs_value *= 10
Copy link

Choose a reason for hiding this comment

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

些末な指摘になりますが、abs_valueを求める部分は1行にまとめてしまっても良いのかなとは思いました 

i += 1
abs_value = 0
while i < len(s) and s[i] in Solution.DIGITS:
digit = ord(s[i]) - ord('0')
Copy link

Choose a reason for hiding this comment

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

digit = int(s[i])と書くこともできるのではないかと思うのですが、何か理由があったりしますか?

Copy link
Owner Author

Choose a reason for hiding this comment

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

大した理由は無くint()でも全然良いんですが、'0'-'9'のUnicode文字1つを数字に直すならこっちの方が多少自然かな?と思ったからですね。

https://docs.python.org/3.11/library/functions.html#ord

Given a string representing one Unicode character, return an integer representing the Unicode code point of that character.

}

private:
void skipWhitespace(const string& s, int& index) const {
Copy link

Choose a reason for hiding this comment

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

C++ の関数名は UpperCamel とすることをお勧めいたします。
https://google.github.io/styleguide/cppguide.html#Function_Names

Ordinarily, functions should start with a capital letter and have a capital letter for each new word.

Copy link
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます、が今のところ、関数名UpperCamelはあまり性に合わないんですよね... (C++のGoogle Style Guide自体正直そんなに合わなそうに感じてます)
規約がそうなってるプロジェクトで書くときは従いますが、そうでなければ別の書き方にするかと思います

int sign = parseSignIfExists(s, index);
while (isdigit(s[index])) {
const int digit = parseDigit(s, index);
if (abs_value > INT_MAX / 10 || abs_value == INT_MAX / 10 && digit > INT_MAX % 10) {
Copy link

Choose a reason for hiding this comment

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

INT_MAX の代わりに std::numeric_limits<int>::max() を使用することをお勧めいたします。理由は、そちらのほうが C++ っぽく見えるから、程度のものです…。

while (s[index] == ' ') {
index++;
}
return;
Copy link

Choose a reason for hiding this comment

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

void 型の関数の末尾の 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.

たしかに。ありがとうございます

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

Successfully merging this pull request may close these issues.

6 participants