Skip to content

20. Valid Parentheses#6

Open
fhiyo wants to merge 3 commits intomainfrom
20_valid-parentheses
Open

20. Valid Parentheses#6
fhiyo wants to merge 3 commits intomainfrom
20_valid-parentheses

Conversation

@fhiyo
Copy link
Copy Markdown
Owner

@fhiyo fhiyo commented May 11, 2024

*/
bool isValid(string s) {
int index = 0;
while (index < s.length()) {
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

@fhiyo fhiyo May 11, 2024

Choose a reason for hiding this comment

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

"()(" こういう入力で コケる 間違えると思います。1回のparseParensは左括弧から対応する右括弧までしかparseしないので。
...とコメントを考えて、上のBNFと合っておらず関数名が悪い気がしました。parseSingleParensとか...?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

あー、すみません。理解しました。parseParens は、1つ目の対応するカッコまでしか consume しないからですね。そうですね。parseParens の中にそれを入れたほうが素直な気がしますがどうでしょうか。

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.

たしかにそう思います。こんな感じですかね

class Solution {
public:
    /*
        parens      := singleParen*
        singleParen := '(' singleParens ')' | '{' singleParens '}' | '[' singleParens ']'
    */
    bool isValid(string s) {
        int index = 0;
        return parseParens(s, index);
    }

private:
    static bool parseParens(const string& s, int& index) {
        while (index < s.length()) {
            if (!parseSingleParen(s, index)) return false;
        }
        return true;
    }

    static bool parseSingleParen(const string& s, int& index) {
        if (index >= s.length()) return false;
        const char left_paren = consume(s, index);
        if (!isLeftParen(left_paren) || index >= s.length()) return false;
        while (!matchPairParen(left_paren, s[index])) {
            if (!parseSingleParen(s, index)) return false;
        }
        consume(s, index); // right paren
        return true;
    }

    static char consume(const string& s, int& index) {
        return s[index++];
    }

    static bool matchPairParen(const char left, const char right) {
        return left == '(' && right == ')' || left == '{' && right == '}' || left == '[' && right == ']';
    }

    static bool isLeftParen(const char ch) {
        return ch == '(' || ch == '{' || ch == '[';
    }
};

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.

いや、 singleParenは singleParen := '(' singleParen ')' | '{' singleParen '}' | '[' singleParen ']' | '' こうか。上のBNFだと無限ループしますね (微妙にtypoもしている)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

singleParen := '(' parens ')' | '{' parens '}' | '[' parens ']'ではないですか?
parseSingleParenのロジックがちょっと複雑な気がしますね。

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.

singleParen := '(' parens ')' | '{' parens '}' | '[' parens ']'ではないですか?

おっしゃる通りでした。自分の書いたのだと (()()) こういうのが通らないですね

public:
ParenParser() : index(0) {}

bool parse(const string& input) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

2回同じstringを入れると、indexが動いてしまっているため、正しく動かないのではないでしょうか?

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.

おっしゃる通りだと思います、ありがとうございます

return parens(input) && index == input.length();
}

bool parens(const string& input) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

parensとconsumeは、privateで良さそうです。

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.

3 participants