Skip to content

48問目 3. Longest Substring Without Repeating Characters#48

Merged
shining-ai merged 3 commits intomainfrom
review48
Jun 29, 2024
Merged

48問目 3. Longest Substring Without Repeating Characters#48
shining-ai merged 3 commits intomainfrom
review48

Conversation

@shining-ai
Copy link
Copy Markdown
Owner

def lengthOfLongestSubstring(self, s: str) -> int:
if not s:
return 0
seen_index = {}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

dict のキーが文字であるという点が、変数名から理解しずらく感じました。 char_to_last_index はいかがでしょうか?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

変数名がsetぽくなっていますね。
分かりやすい変数名のご提案ありがとうございます。

Copy link
Copy Markdown

@hayashi-ay hayashi-ay left a comment

Choose a reason for hiding this comment

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

良いと思います。

def lengthOfLongestSubstring(self, s: str) -> int:
max_length = 0
for i, base_c in enumerate(s):
seen = set(base_c)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

自分ならここの処理も後続のfor分で合わせてやるかなと思いました。

length = 0
for c in s[i:]:

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

0始まりのほうが分かりやすいですね。

Comment on lines +24 to +25
if not s:
return 0
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

特別扱いしなくても良いかなと思います。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

空文字の時、rightが存在しなくてエラーになってしまうんですよね。

right = -1

の初期化だと意図がつかめないかと思い、not sをチェックしました。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

なるほど、ちゃんと読めてなかったです。for文で定義した変数をその外で使うのが良くないと思うんですよね。Pythonの言語仕様的には問題ないですが。

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.

3 participants