Conversation
| @@ -0,0 +1,31 @@ | |||
| class Solution { | |||
There was a problem hiding this comment.
想定していたアルゴリズムとまったく違うアルゴリズムで、違和感を感じました。
- current の末尾に「(」または「)」を付け加える。
- current の末尾から「(」または「)」を取り除く。
の操作だけで書けませんか？途中、括弧を閉じすぎたり、最後に開きすぎたりしないかのチェックは必要だと思います。
There was a problem hiding this comment.
ありがとうございます！提案いただいた方法で実装してみました！アプローチや思考などはREADMEに記載していています！
|
あっ２つ目の解法の部分で今気づきましたが、途中でのチェックの関数と最終チェックの関数について最後以外の処理は一緒なので、引数でinProcess: boolanとかを持たせてもいいかもですね |
| } | ||
|
|
||
| private boolean isValidParenethesisInProcess(String string) { | ||
| int leftParenethesisCount = 0; |
There was a problem hiding this comment.
leftParenethesisCount という変数名ですと、「(」の数を表しているように感じます。変数の中に含まれている値が表すものを、より直接的に表現した変数名にできますか？
There was a problem hiding this comment.
ここは、おっしゃるとおり「(」の数を表していた：（正確にいうと文字列の中の"（"をカウントする変数）のですが、僕の認識違いですかね？
There was a problem hiding this comment.
「「(」の数」では不正確なように思います。「まだ閉じられていない、開いている「(」の数」ではないでしょうか？
There was a problem hiding this comment.
なるほど！現在の状態も正確に表すように含めるということですね！ありがとうございます！
There was a problem hiding this comment.
ちなみにこれは少し脱線気味ですが、関数名と変数名の意味の含み度合いについて目安などありますでしょうか？
今回の例だと、isValidParenethesisInProcessに、途中経過という意味が入っているので完全に閉じた関係ではないことも少し意味的に含んでいるようにもとらえられるかもと思いました（可読性の観点ではもちろん、一眼でわかったほうがいいので状態も含めたほうがいいのはもちろんなのですが）
プロジェクトだと大体こういう場合はコメントをつけて補足することが多いので気になった次第です！
There was a problem hiding this comment.
変数に含まれる値や、関数の内容が、必要十分に分かる端的な英単語・英語句を付けるのが良いと思います。
| return parenthesisStrings; | ||
| } | ||
|
|
||
| private boolean isValidParenethesisInProcess(String string) { |
There was a problem hiding this comment.
generateParenthesis() の中で文字列を生成する際、同時に leftParenethesisCount も持たせれば、 isValidParenethesisInProcess() と isValidParenethesis() が不要になると思います。
異なる処理を分割したほうが実装がシンプルになることはあります。ですが、今回のケースでは処理内容が十分に単純なため、同時に行ったほうが良いと思います。
| } | ||
|
|
||
| private boolean isValidParenethesisInProcess(String string) { | ||
| int notClosedleftParenethesisCount = 0; |
There was a problem hiding this comment.
nit: notClosedleftParenethesisCount -> notClosedLeftParenethesisCount
|
|
||
| while (!parenthesisCandidate.isEmpty()) { | ||
| String current = parenthesisCandidate.poll(); | ||
| int currentLeftParenthesisCount = unClosedleftParentheses.poll(); |
There was a problem hiding this comment.
currentLeftParenthesisCount と、接尾辞に Count を付けても良いのですが、接頭辞に numberOf を付けるやり方盛ります。通例、 num と略して書くと思います。
numCurrentLeftParenthesis
この辺りは、チーム内で主流なやり方を調べ、それに合わせるのが良いと思います。また、複数の書き方ができるようにしておき、どれでも対応できるようにしておくとよいと思います。
| class Solution { | ||
| public List<String> generateParenthesis(int n) { | ||
| List<String> allParentheses = new ArrayList<>(); | ||
| Queue<String> parenthesisCandidate = new LinkedList<>(); |
There was a problem hiding this comment.
ありがとうございます！こちらはStringとIntegerを変数にもつクラスを定義して、そのオブジェクトをQueueに入れていくというイメージであっていますでしょうか？
|
|
||
| class ParenthesisState { | ||
| String str; | ||
| int numUnClosedLeftParenthesis; |
There was a problem hiding this comment.
unclosed で 1 単語だと思います。 numUnclosedLeftParenthesis
| public List<String> generateParenthesis(int n) { | ||
| List<String> allParentheses = new ArrayList<>(); | ||
| Queue<Pair<String, Integer>> parenthesisCandidate = new LinkedList<>(); | ||
| parenthesisCandidate.add(new Pair<>("", 0)); |
There was a problem hiding this comment.
Pair は、コードを読むにあたり、中に含まれている値を把握するのに認知負荷がかかるため、あまり使わないほうが良いと思います。代わりにユーザー定義クラスを使ったほうが良いと思います。
| allParentheses.add(currentState.getKey()); | ||
| continue; | ||
| } | ||
| for (int i = 0; i <= numCurrentUnClosedLeftParenthesis + 1; i++) { |
There was a problem hiding this comment.
「)」をまとめて複数個追加するメリットはないように感じます。「(」または「)」を 1 文字ずつ追加すれば十分だと思います。
4 participants
・問題URL
https://leetcode.com/problems/generate-parentheses/
・課題/感想
ACはできたのですが、途中でおなじことの繰り返しによるTLEがでたので前の状態を保持するようにして解消しました。
ただ、時間計算量の見積もりができなくてなやんでいます