-
Notifications
You must be signed in to change notification settings - Fork 0
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
base: main
Are you sure you want to change the base?
Conversation
- https://github.com/Mike0121/LeetCode/pull/23 | ||
|
||
digits部分をまとめてから一気に変換している。先に `s = s.lstrip()` するのもなるほど。 | ||
超巨大な数になる場合、最後までparseを止めないため不利になる実装。 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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'への対応が一箇所で済みますね。
(ただ、このコードでそこまでやるか?は微妙なところですが)
There was a problem hiding this comment.
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)
が無難かな...
There was a problem hiding this comment.
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)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1文字の文字列の配列を作ってからjoin()するのはコストがかかりそうなので、この場合はindexを使って部分文字列を取るほうがよいですかね。あと別の方法としては正規表現で置換するなどもあります。
There was a problem hiding this comment.
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
こういう感じですかね
全体的に良いと思いました。3rdが一番読みやすく感じました。 |
s[index] == '6' or \ | ||
s[index] == '7' or \ | ||
s[index] == '8' or \ | ||
s[index] == '9' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s[index] in "0123456789" という書き方があります。
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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' |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
digit = int(s[i])
と書くこともできるのではないかと思うのですが、何か理由があったりしますか?
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
void 型の関数の末尾の return
は意味がないため、書かないほうが良いと思います。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
たしかに。ありがとうございます
https://leetcode.com/problems/string-to-integer-atoi/description/