Skip to content

Conversation

shintaroyoshida20
Copy link
Owner

@shintaroyoshida20 shintaroyoshida20 commented Apr 7, 2025

問題URL

https://leetcode.com/problems/valid-parentheses/

問題文

Given a string s containing just the characters '(', ')', '{', '}', '[' and ']', determine if the input string is valid.

An input string is valid if:

Open brackets must be closed by the same type of brackets.
Open brackets must be closed in the correct order.
Every close bracket has a corresponding open bracket of the same type.

@shintaroyoshida20 shintaroyoshida20 force-pushed the feature/stack/valid-parentheses branch from c21d6af to b02e347 Compare April 7, 2025 23:52
@shintaroyoshida20 shintaroyoshida20 self-assigned this Apr 9, 2025
@shintaroyoshida20 shintaroyoshida20 marked this pull request as ready for review April 17, 2025 00:56
Copy link

@huyfififi huyfififi left a comment

Choose a reason for hiding this comment

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

良いと思います! 👍

container.push(character)
continue
}
const converted_last_character = open_to_close.get(container.pop())
Copy link

@huyfififi huyfififi Apr 17, 2025

Choose a reason for hiding this comment

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

nit データをある形から違う形に変換 (convert) している、というよりかは対応する括弧をとってきているので、converted_*よりは expected_closingのようにして if (character == expected_closing) {としてしまう方がわかりやすく感じました。

Copy link
Owner Author

Choose a reason for hiding this comment

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

変数名と内容が合致していなかったですね!

以下の変数名もあるよと、ChatGPTが教えてくれました。

  • corresponding_*
  • matching_*

return false
}
}
return container.length === 1 && container[container.length-1] === "SENTINEL"
Copy link

@huyfififi huyfififi Apr 17, 2025

Choose a reason for hiding this comment

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

このようなやり方もあるんですね、興味深いです 💡
これは個人的な感想ですが、このような 尖兵? 番兵の値がどのような処理 (doesMatchBracket)を通っていくのか想像するのがやや難しく感じるので、読む際に覚えることを減らす意味でも line 274 で if (container.length === 0) return falseとreturn earlyした方がわかりやすいなと思います。が、私の感覚が一般のものであるかどうかは自信がありません。

Copy link
Owner Author

Choose a reason for hiding this comment

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

@huyfififi

mapを使った場合には、Mapのkeyでundefinedが入るケースを検討しないといけなかったのでreturn earlyが有効だと思ったのですが、

今回だとdoesMatchBracketに SENTINELが渡されるケースでfalseが返却されることは自明なので、僕の意見としてはどちらでも良いのかなと思いました。

* 机の上に何か残っている場合には、異常を報告する。

```
var isValid = function(characters) {
Copy link

@huyfififi huyfififi Apr 17, 2025

Choose a reason for hiding this comment

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

"aaa"みたいな文字列が入力された際にtrueを返しそうですね。LeetCodeではそこまで予期せぬ入力について考えなくても良いでしょうし、JavaScriptのベストプラクティスはよくわからないのですが、予期せぬ値が来た時に true or falseを返してしまうと、本当は意図しない動作なのに関数を呼び出した側からしたらうまくいったように見えてわかりづらいので、業務ではエラーを吐いた方が逆に親切な気がします。

Copy link
Owner Author

@shintaroyoshida20 shintaroyoshida20 Apr 20, 2025

Choose a reason for hiding this comment

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

38c934f でエラーハンドリングのコードを追加しました

}
}

if (container.length === 0) {
Copy link

@huyfififi huyfififi Apr 17, 2025

Choose a reason for hiding this comment

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

return container.length === 0の方が、同じ条件を表現できている上、4行も読まなくてもいいので個人的にはわかりやすく感じます

Copy link
Owner Author

Choose a reason for hiding this comment

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

僕も同感です!

container.push(character)
continue
}
const open_bracket_candidate = container.pop()
Copy link

@huyfififi huyfififi Apr 17, 2025

Choose a reason for hiding this comment

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

ご自身でご指摘の通り、containerが空の時open_bracket_candidate = undefinedとなって、それでも続く処理 (doesMatchBracket)は問題なく進むのでしょうが、open_bracket_candidatestringである時とundefinedである時の2パターンの型を考えなければいけないのが、個人的にはやや負荷に感じました。containerが空だったとき早めにfalseを返してしまう方が親切かもしれません。

Copy link
Owner Author

Choose a reason for hiding this comment

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

確かに早めにreturnする方が脳の負荷が減りますね。

ee3aaa5

};
```

* `*2` 番兵をおく方法
Copy link

Choose a reason for hiding this comment

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

Python の場合は、空であったときに pop すると undefined が返らずに IndexError になります。
JavaScript は、Map も undefined が返るので、自動的に番兵になっているともいえるかと思います。

Copy link
Owner Author

Choose a reason for hiding this comment

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

コメントありがとうございます。

今後は、スクリプト言語のPythonの仕様も確認して、言語間の違いも意識しようと思います。

["(", ")"]
])
const container = []
const expected_characters = ["(", ")", "{", "}", "[", "]"]

Choose a reason for hiding this comment

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

些細ですが、エラーメッセージで"Invalid character"と書いてあるので、変数名もvalid_charactersでよいのかなと思いました!

Copy link
Owner Author

Choose a reason for hiding this comment

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

invalidかどうかの判定をするための配列なので、valid_charactersの方がexpected_charactersよりも読みやすいですね!

var isValid = function(characters) {
const container = []
const open_bracket_chars = ["(", "{", "["]
for (const character of characters) {

Choose a reason for hiding this comment

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

bracketなどの命名でもいいのかなと思いました。
characterだと予約語として存在しそうで、引っかかりました。
※Javaには大文字ですが、Characterクラスが存在しております。

Copy link
Owner Author

@shintaroyoshida20 shintaroyoshida20 May 6, 2025

Choose a reason for hiding this comment

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

予約語を変数名として避けるという意識ができておりませんでした。ありがとうございます。
確かに、charactersにはbracketの文字が入るという前提条件があるので、bracketでも良いですね。

@nodachipのコメントだと、C++の予約語を避けて、charを避けて ch, c, characterを選ぶというコメントを見つけました。言語によってもよく使う変数名があるというのが気付きでした。

以下がJavasciprtの予約語だそうです。

await break case catch class const continue debugger default
delete do else enum export extends false finally for function if
import in instanceof new null return super switch this throw true try typeof var void while with yield

https://262.ecma-international.org/#sec-keywords-and-reserved-words

Javaだと charも予約語なのですね。
https://docs.oracle.com/javase/tutorial/java/nutsandbolts/_keywords.html

return false
}
}
if (container.length === 0) {

Choose a reason for hiding this comment

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

return container.length === 0でいいかなと思いました。

Copy link
Owner Author

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants