Skip to content

Conversation

hroc135
Copy link
Owner

@hroc135 hroc135 commented Aug 1, 2024

numToIdx[n] = i
}

log.Panic("No valid pair found.")
Copy link

Choose a reason for hiding this comment

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

シグネチャを変える必要があるのでleetcode上では出来ない気がしますが、Goではエラーを返すことが一般的かと思います。
また、その場でプログラム全体を停止させたい場合は、例えば以下のように書けますね。

log.Fatal("No valid pair found.")
panic("unreachable")

panic自体はスレッド(正確にはゴルーチン)を停止させます。

Copy link

Choose a reason for hiding this comment

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

Fatal は、os.Exit 呼ぶので、次の行の panic は不要ではないでしょうか。
https://pkg.go.dev/log#Fatal

Copy link

Choose a reason for hiding this comment

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

こちらの panic はコンパイラに unreachable であることを伝えるために置いています。
この用法については以下に記述があります。
https://google.github.io/styleguide/go/best-practices.html#when-to-panic

Panic is also used when the compiler cannot identify unreachable code, for example when using a function like log.Fatal that will not return:

Copy link

Choose a reason for hiding this comment

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

なるほど、ありがとうございます。

func twoSum(nums []int, target int) []int {
numToIdx := make(map[int]int)
for i, n := range nums {
dif := target - n

Choose a reason for hiding this comment

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

この変数は多分 difference の略語だと思うのですが、diff とするのが一般的だと思います

numToIdx := make(map[int]int)
for i, n := range nums {
dif := target - n
if _, exist := numToIdx[dif]; exist {
Copy link

Choose a reason for hiding this comment

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

Goではexistではなくokを使用するのが一般的かなと思いました

Copy link
Owner Author

Choose a reason for hiding this comment

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

okはサンプルコードでよく見かけましたが、これで伝わるのかなと思ってしまい、existを使っていました。でも、慣習には従った方が良さそうですね

for i, n := range nums {
dif := target - n
if _, exist := numToIdx[dif]; exist {
return []int{i, numToIdx[dif]}
Copy link

Choose a reason for hiding this comment

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

numToIdx[dif]ではなく、76行目でif j, ok := numToIdx[dif]; ok {などのようにしてあげるとreturn []int{i, j}とすっきり書けます

Copy link

Choose a reason for hiding this comment

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

そうするとdifが使われる箇所が一箇所だけになるので、dif自体の宣言をやめて、76行目でそのままnumToIdx[target-n]のようにしてしまっても良いかなと思いました。

Copy link
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます!おっしゃる通りです

@rihib rihib mentioned this pull request Aug 6, 2024
- こういった身体性を持つというか、実世界に例えてみる想像力はアルゴリズムを思いつくときに非常に有用だなと思った
- 答えが存在しない場合にpanicするコードを書いている人を見かけたので、自分もやってみることに
- 以前からleetcodeをやっているとエラーハンドリングについてのセンスが磨かれないなという点を問題点だと思っていた

Copy link

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.

5 participants