Skip to content

Create 20. Valid Parentheses.md#19

Open
Apo-Matchbox wants to merge 1 commit intomainfrom
20.-Valid-Parentheses
Open

Create 20. Valid Parentheses.md#19
Apo-Matchbox wants to merge 1 commit intomainfrom
20.-Valid-Parentheses

Conversation

@Apo-Matchbox
Copy link
Owner

Comment on lines +128 to +131
std::array<char, 128> close_to_open{};
close_to_open[')'] = '(';
close_to_open['}'] = '{';
close_to_open[']'] = '[';

Choose a reason for hiding this comment

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

map ではなく array にしたのは意図的でしょうか。小さいものなら単純な array の方が効率がよい、というのはあるかもしれませんね。

Copy link
Owner Author

Choose a reason for hiding this comment

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

@ryosuketc
ご返信が随分と遅くなり大変申し訳ございません。

こちらを書いた時は、「弧の種類(3種)が少なく、拡張を考えなくていいから、速さとシンプルさを優先して array を選んだ」形ですね。
新たに括弧を追加する場面も考えると、unordered_mapで書いた方がいいかもしれないですね。

->開き括弧を確認後、対応する閉じ括弧そのものをスタックに積む。閉じ括弧が来たらスタックの先頭と一致するかだけを見るので、マップも比較分岐も最小限で済ませる。

- [配列テーブル(closing→opening)+文字列をスタックとして使う] (別解2)
->unordered_map の代わりに 固定長配列(ASCII 128) を使って 閉じ括弧→対応する開き括弧 を O(1) 参照。スタックは std::string を使うと軽量(push/pop_back)。

Choose a reason for hiding this comment

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

スタックは std::string を使うと軽量(push/pop_back)。

この発想はありませんでした。ただ std::stack と比べて可読性が下がるように感じます。。

Copy link

Choose a reason for hiding this comment

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

ほぼ変わらない気がします。> 軽量

return false;
}

std::array<char, 128> close_to_open{};
Copy link

Choose a reason for hiding this comment

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

{} を付けると 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.

@nodchip
ご返信が随分と遅くなり大変申し訳ございません。
ご指摘頂きありがとうございます。

ここでは、'(', '{', '['の3つだけなので、後々代入するのであれば、初期化なしにした方がいいかもしれないですね。
色々と調べた時に、未代入のインデックスにアクセスすると 未定義動作(UB)になる可能性がある(https://en.cppreference.com/w/cpp/language/ub.html)と記載があったので、初期化をしてました。

順に書いていった時に、必要・不必要なこと当たり前に書けるようにします。

public:
bool isValid(string& s) {
unordered_map<char, char> open_to_close = {
{'(', ')'},
Copy link

Choose a reason for hiding this comment

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

インデントがずれているようです。

std::stack<char> st;

for (char c : s) {
switch (c) {
Copy link

Choose a reason for hiding this comment

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

好みの範囲かもしれませんが、括弧の種類が増えたときの拡張性なども考えると括弧はmap等で管理し、ifで分岐処理を記述したほうが簡潔なように思います。

Copy link
Owner Author

Choose a reason for hiding this comment

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

@maeken4
ご返信随分と遅くなり大変申し訳ありません。
ご指摘ありがとうございます。他のパターンを意識した記載ができておりませんでした。
問題文の範囲外でも一般的に考えられる所まで気をつけたいと思います。

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