Skip to content

49. Group Anagrams#15

Open
fhiyo wants to merge 2 commits intomainfrom
49_group-anagrams
Open

49. Group Anagrams#15
fhiyo wants to merge 2 commits intomainfrom
49_group-anagrams

Conversation

@fhiyo
Copy link
Copy Markdown
Owner

@fhiyo fhiyo commented Jun 1, 2024

class Solution:
def groupAnagrams(self, strs: List[str]) -> List[List[str]]:
sorted_str_to_anagrams = defaultdict(list)
for str_ in strs:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

普通の変数名でtrailing underscoreは一般的ですか?

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.

https://peps.python.org/pep-0008/#descriptive-naming-styles

In addition, the following special forms using leading or trailing underscores are recognized (these can generally be combined with any case convention):
(省略)
single_trailing_underscore_: used by convention to avoid conflicts with Python keyword, e.g. :

tkinter.Toplevel(master, class_='ClassName')

なので、keywordとの衝突を避けるときに使うのは一般的だという認識です。ただstrはkeywordではなくbuilt-in functionなので正確には違うんですが、そこはいいんじゃないかな?と解釈しています

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.

私は、これくらい明らかならば、一文字変数でも別にいいんじゃないかな

自分もそう思います。今回は単数形と複数形の対比がはっきりしていると思ったのでこちらを選びました。自分が考えるよりも他の方のtrailing underscoreへの抵抗感は強そうだと感じました...

https://peps.python.org/pep-0008/#function-and-method-arguments

Perhaps better is to avoid such clashes by using a synonym.

とあるし、もう少し別の語を使って衝突を避ける方向で行こうと思います

class Solution:
def groupAnagrams(self, strs: List[str]) -> List[List[str]]:
def counting_sort(s: str) -> str:
counts = [0] * 26 # a-z
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

26は定数として定義しちゃう考えもあるかなと思いました。

return ''.join(map(lambda t: chr(ord('a') + t[0]) * t[1], enumerate(counts)))

sorted_str_to_anagrams = defaultdict(list)
for str_ in strs:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

素人質問ですみません。str_のアンダースコアは、どういった意味合いで使用されてるのでしょうか?

Copy link
Copy Markdown
Owner Author

@fhiyo fhiyo Jun 2, 2024

Choose a reason for hiding this comment

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

strという名前だと build-in関数と被るので、アンダースコアを付けて回避してます。

#15 (comment)

(追記)
訂正
誤: build-in関数, 正: built-in関数

Copy link
Copy Markdown

@t-ooka t-ooka Jun 2, 2024

Choose a reason for hiding this comment

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

既に議論あったの見逃してて、申し訳ありません!

build-in関数との衝突を避ける意図であればsだけとかでもシンプルでいいかなと思いました。

Comment on lines +43 to +44
for ch in s:
counts[ord(ch) - ord('a')] += 1
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

文字列がsなのに対して、文字がchなのは一貫性のなさを少し感じました。
cでもよいのかなと個人的には思います。

Comment on lines +148 to +157
groupby()とextend()を使っても良いかも。

```py
class Solution:
def groupAnagrams(self, strs: List[str]) -> List[List[str]]:
sorted_to_anagrams = defaultdict(list)
for key, g in groupby(strs, key=lambda s: ''.join(sorted(s))):
sorted_to_anagrams[key].extend(g)
return list(sorted_to_anagrams.values())
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

勉強になりました!
ところでこちらはitertoolsのgroupby関数ですよね?

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.

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

Choose a reason for hiding this comment

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

あと、これで動きます? sort 要りません?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

あ、失礼誤読でした。しかし、これだと groupby している意味があんまりないですね。

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.

これならsortしなくてもgroupby動くなと思って書いたんですが、意味が薄いと言われるとそれも確かに。
sortするコストを考えると使いどころが難しいイメージがあります

counts = [0] * 26 # a-z
for ch in s:
counts[ord(ch) - ord('a')] += 1
return ''.join(map(lambda t: chr(ord('a') + t[0]) * t[1], enumerate(counts)))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

私だけかもしれませんが、ここの処理の理解に少し時間を要しました。
(と言いつつ代替案がないのですが)

そういう観点で、tuple返した方が分かりやすいかもしれないと思いました。

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