-
Notifications
You must be signed in to change notification settings - Fork 0
Solved: 3. Longest Substring Without Repeating Characters #50
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
ryosuketc
left a comment
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: | ||
| def lengthOfLongestSubstring(self, s: str) -> int: | ||
| max_len = 0 | ||
| ascii_last_used_index = [-1] * 128 |
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.
この問題には関係ないですが、ASCII なんで (8ビットではなく) 7ビットなんだろうと思ったらこういうことだったんですね。
https://ja.wikipedia.org/wiki/ASCII
ASCII制定当時、最小のデータ処理単位(メモリアドレッシングの最小単位)つまりバイトが6ビットであるコンピュータも多かった(DECのPDPシリーズなど)。そのようなコンピュータでは6ビットの文字符号化方式を採用しており、そのためISO/IEC 646の策定にあたっては、7ビット符号化案の他に6ビット符号化案もあった。のちに1バイトを8ビットとみなす、つまりオクテットを採用するコンピュータが人気となり、主流となっていった[4]。オクテットを採用したコンピュータでASCIIを扱う場合、1ビットの余りがあるので、その8ビット目は通信におけるエラーチェック用のパリティビットとして用いられていた[3]。
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.
コメントありがとうございます。
結構、揺れ動いていたのですね。
直近、SJIS、Windows_31J、Shift_JIS、CP932、MS932あたりの整理に詰まったことがあり、文字コード周りはある程度、歴史的背景から追わないとなあとなりました。
| def lengthOfLongestSubstring(self, s: str) -> int: | ||
| max_len = 0 | ||
| ascii_last_used_index = [-1] * 128 | ||
| window_start = 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.
接頭辞の window は不要だと思いました。
ダイクストラを実装するときに dijkstra_src という変数名はつけないと思います
| ```py | ||
| class Solution: | ||
| def lengthOfLongestSubstring(self, s: str) -> int: | ||
| def is_last_index_in_window(character: str) -> bool: |
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.
インナー関数を使うメリットがあまり感じられませんでした。インナー関数を定義するメリットの一つは同じ処理をひとまとめにできることだと思いますが、この場合は一箇所で呼ばれているだけですね。それから個人的には20行程度の関数であれば上から下に読みたいのですが、インナー関数があるとどうしても目を上下せざるを得なくなるのも難点かと思います。
あとこのインナー関数はループでどういう処理をしているか知らないとピンとこないと思います。なんというか、インナー関数を補助的に使いたいはずなのに、関数の肝?となる部分(文字が最後に見つかった位置がスライディングウィンドウの左端より前にあるか後ろにあるかで挙動を変える)が含まれているのが微妙に可読性を損なわせているのかもしれません。
長々と書いてケチをつけてしまいましたが、わざわざインナー関数書かなくてもいいのでは、という提案です。
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.
if文が横に長くなって見づらいかと思いインナー関数を定義しましたが、
おっしゃる通り、かえって分かりにくくなっている気がするので、ない方が適切でしたね。
ご指摘ありがとうございます。
3. Longest Substring Without Repeating Characters