Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions 01-15/11.Valid-Parentheses/1.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
#if __has_include("../../debug.hpp")
#include "../../debug.hpp"
#endif
// ここまでローカルでのデバッグ用なので気にしないでください --------------------

#include <map>

using namespace std;

// <時間>
// 10分
// <感想>
// これ正直10分じゃ無理だろ、と思っていたが意外と書けて嬉しい。
// 最初の方でどうするのがいいか少し時間を取って考えた。
// 最初に思い付いたのがpre変数を用意して、
// ひとつ前の文字に対応する括弧のみ受けつけ続けるみたいな(結局うまく行かなそう)。
// そっから何かの拍子にstackを使うことを思いつき、
// 途中でclosing_to_openingが必要になり
// switchで書いていたロジックをclosing_to_openingを使った
// ifに書き換えるという流れ
// <疑問・不安点(・個人的な感想、違う意見があれば教えてほしいもの)>
// - L49を空けるか否か
// - closing_to_openingとopening_bracketsのどちらを先に宣言するか
Comment on lines +22 to +23
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.

ありがとうございます!
なんか今更なんですが、色々こうやってレビューもらっているうちに、プログラミングの可読性の面は言語ゲームなんじゃないかと思い始めてきました。
またなんかすごいブレークスルーした気がします。

// (細かいけど...)
// <他の人のを見てコメント>
// - なるほど。そういえばValid ParenthesesもCSZAPで触れていたかもしれない。
// https://discord.com/channels/1084280443945353267/1201211204547383386/1202541275115425822
// - そうか、closing_to_openingじゃなくてもopening_to_closingでもよかったのか…
// うーん、でもその場合2回map::findすることになるな
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

直感、あんまり速度が問題にならないと思うので、私はこれは気にしません。

ボトルネックでないところを最適化しない話がこのあたりに。
https://docs.google.com/document/d/11HV35ADPo9QxJOpJQ24FcZvtvioli770WWdZZDaLOfg/edit?tab=t.0#heading=h.agkea65vtdhp

// - stackに番兵を入れてstack::emptyを呼び出さなくていいようにしている
// ものがあった。頭いい。
// - closing_to_opening定数だからスネークケースじゃなかったわ。
//
//
//
//
//
//
//
// L40(行数調整用コメント)
class Solution {
public:
bool isValid(string s) {
static const map<char, char> closing_to_opening{
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

関数内で static 変数を定義した場合、最初に関数が呼ばれたときに初期化されます。その際、競合状態を防ぐためにロックが取られます。結果、関数の呼び出し毎にロックが取られ、パフォーマンスが低下する場合があります。原則、関数の外で定義することをお勧めします。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

static 変数のロックは C++11 からそうなっているんですね。
https://cpprefjp.github.io/lang/cpp11/static_initialization_thread_safely.html

{')', '('},
{'}', '{'},
{']', '['},
};

stack<char> opening_brackets;
for (auto c : s) {
if (auto it = closing_to_opening.find(c);
it != closing_to_opening.end()) {
auto opening = it->second;
if (!opening_brackets.empty() && opening_brackets.top() == opening) {
opening_brackets.pop();
} else {
return false;
}
} else {
opening_brackets.push(c);
}
Comment on lines +52 to +62
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

if else のネストが分かりにくいと私は思います。このあたりをざっと見ておくといいかもしれません。
https://docs.google.com/document/d/11HV35ADPo9QxJOpJQ24FcZvtvioli770WWdZZDaLOfg/edit?tab=t.0#heading=h.9kpbwslvv3yv

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.

auto itを同じブロックで処理することに気を取られてましたが別に条件式外にすればネストを解消できましたね...
ありがとうございます!

}
return opening_brackets.empty();
}
};
35 changes: 35 additions & 0 deletions 01-15/11.Valid-Parentheses/2.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#if __has_include("../../debug.hpp")
#include "../../debug.hpp"
#endif
// ここまでローカルでのデバッグ用なので気にしないでください --------------------

#include <map>

using namespace std;

// <時間>
// 5分
// <感想>
class Solution {
public:
bool isValid(const string &brackets) {
static const map<char, char> kClosingToOpening{
{')', '('},
{'}', '{'},
{']', '['},
};
stack<char> opening_brackets;
opening_brackets.push('*'); // sentinel
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

'*' 提案したのは自分な気がしますが、'\0' あたりのほうが自然かもしれません。

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'いいですね!ありがとうございます!

for (auto bracket : brackets) {
if (auto it = kClosingToOpening.find(bracket);
it != kClosingToOpening.end()) {
auto opening = it->second;
if (opening_brackets.top() != opening) return false;
opening_brackets.pop();
} else {
opening_brackets.push(bracket);
}
}
return opening_brackets.size() == 1;
}
};
49 changes: 49 additions & 0 deletions 01-15/11.Valid-Parentheses/3.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
#if __has_include("../../debug.hpp")
#include "../../debug.hpp"
#endif
// ここまでローカルでのデバッグ用なので気にしないでください --------------------

#include <map>
#include <stack>
#include <string>

using namespace std;

// <時間>
// 15分
// <コメント>
// - corresponding_bracketとopen_bracketは最初逆に名前を付けていた。
// - corresponding_bracketは最初corresponding_open_bracketとしていたが、
// openはなくても伝わると思い消した。
// - だが、読む側に取ってどう見えるかは分からない。
// - correspondingみたいな意味の名前は付けることが多いと思うが、
// もっと短めの単語はないものか。
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

他の候補として、私は pair や match を連想しました。

class Solution {
public:
bool isValid(const string &brackets) {
static const map<char, char> kCloseToOpen{
{')', '('},
{'}', '{'},
{']', '['},
};
stack<char> open_brackets;
open_brackets.push('\0');
for (auto bracket : brackets) {
// In the case `bracket` is an open bracket.
auto close_and_open = kCloseToOpen.find(bracket);
if (close_and_open == kCloseToOpen.end()) {
open_brackets.push(bracket);
continue;
}

// In the case `bracket` is a close bracket.
auto open_bracket = close_and_open->second;
auto corresponding_bracket = open_brackets.top();
open_brackets.pop();
if (corresponding_bracket != open_bracket) {
return false;
}
}
return open_brackets.size() == 1;
}
};
48 changes: 48 additions & 0 deletions 01-15/11.Valid-Parentheses/4.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
#if __has_include("../../debug.hpp")
#include "../../debug.hpp"
#endif
// ここまでローカルでのデバッグ用なので気にしないでください --------------------

#include <map>
#include <stack>
#include <string>

using namespace std;

// <時間>
// 10分
// <感想>
// 多分3.cppよりこっちの方が読みやすい。
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

同感です。OpenToCloseに入っているかを調べる方が直感的だと思いました。
自分の理解力の問題だと思いますが、CloseToOpenにない、と言われると逆の逆みたいに感じて理解しにくかったです。

// <コメント>
// - auto open_bracket = open_brackets.top();
// で新しく変数を作ろうかとも思ったが、無駄に変数が多くても
// 読みづらいかなと思い作らないことにした。
class Solution {
public:
bool isValid(const string &brackets) {
static const map<char, char> kOpenToClose{
{'(', ')'},
{'{', '}'},
{'[', ']'},
};
stack<char> open_brackets;
for (auto bracket : brackets) {
// In the case `bracket` is an open bracket.
if (kOpenToClose.contains(bracket)) {
open_brackets.push(bracket);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

これは好みかなと思いますが、わかりやすさ(正しさの保証)でいうと、kOpenToClose.at()をここでやっている方が親切な気がしますね。(containsが必ず保証されていることがすぐわかる、kOpenToCloseをこの後で使わなくなるので構造的な見通しが良くなる)

ただ、エラーケースの計算量がちょっと減るので、シビアな計算のことを考えるならば計算をなるべくあとに回す習慣でも良いかもしれません。

continue;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

私の好みとなり恐縮ですが、コメントを残されているので、この空白行はなくても読みやすく感じました。

// In the case `bracket` is a close bracket.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

この問題の前提となる入力の場合はここのコメント通りなんですが、他の文字が入った場合の関数の挙動としては厳密には違うので、入力に対する前提知識がないと微妙に誤った理解を与えるかもしれません。
// In the case where bracket should be a closing bracket
とかですかね。(英語でどうコメントするのがいいかわからなくて、ChatGPTに聞きました)

if (open_brackets.empty()) {
return false;
}
auto close_bracket = kOpenToClose.at(open_brackets.top());
open_brackets.pop();
if (bracket != close_bracket) {
return false;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

open_brackets.pop();はカッコの不一致を判定した後(L44 直後)に来た方が個人的には分かりやすく感じます。
「カッコの種類が一致したらポップ」という考えだと思うので、
「カッコの種類が一致しないかを判定し、それ以外の場合はポップ」
という流れの方が分かりやすいかなと思いました。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

close_bracketをわざわざ宣言せず

if (bracket != kOpenToClose.at(open_brackets.top())) {

としても十分わかりやすいと思います。
kOpenToCloseの名前からOpenをキーとするとCloseが返ってくることが分かり、名前からopen_brackets.top()がOpenであることも分かるので、kOpenToClose.at(open_brackets.top())という形がCloseであることは分かりやすいと思うからです。

}
return open_brackets.empty();
}
};
3 changes: 3 additions & 0 deletions 01-15/11.Valid-Parentheses/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# 20. Valid Parentheses

<https://leetcode.com/problems/valid-parentheses/description/>