Conversation
Implemented intersection method using HashSet to find common elements between two arrays. Optimized for space by using the shorter array for the set.
|
拡張子を .md にする必要があるのかなと思いました。 先に進むと見えてくるところがあると思います。 |
レビューを反映して、コードブロックが表示されるよう.mdファイルでコミット
レビューいただきありがとうございます。 |
naoto-iwase
left a comment
There was a problem hiding this comment.
よく検討されていて素晴らしいと思います。
コードも読みやすかったです。
| short_nums.add(num2); | ||
| } | ||
| long_nums = nums1; |
There was a problem hiding this comment.
Javaの変数名はsnake_caseよりcamelCaseの方が一般的だと思います。
short_nums -> shourtNums
There was a problem hiding this comment.
言語によって、一般的とされる命名規則が違うのですね。
勉強になりました。
今回はもともとpythonのコードを参考にしたので、そのままsnake_caseで書いていたのですが、
次はJavaでより一般的なcamelCaseで書くようにしたいと思います。
There was a problem hiding this comment.
参考までにスタイルガイドへのリンクを共有いたします。
https://google.github.io/styleguide/javaguide.html#s5.2.7-local-variable-names
Local variable names are written in lowerCamelCase.
なお、このスタイルガイドは“唯一の正解”というわけではなく、数あるガイドラインの一つに過ぎません。チームによって重視される書き方や慣習も異なります。そのため、ご自身の中に基準を持ちつつも、最終的にはチームの一般的な書き方に合わせることをお勧めします。
| int[] long_nums; | ||
|
|
||
| if (nums1.length > nums2.length) { | ||
| for (int num2 : nums2) { | ||
| short_nums.add(num2); | ||
| } | ||
| long_nums = nums1; | ||
| } | ||
| else { | ||
| for (int num1 : nums1) { | ||
| short_nums.add(num1); | ||
| } | ||
| long_nums = nums2; | ||
| } |
There was a problem hiding this comment.
テクニカルなんですが、下記のように書く方法もあります。
class Solution {
public int[] intersection(int[] nums1, int[] nums2) {
if (nums1.length > nums2.length) {
return intersection(nums2, nums1);
}
// 以降はnums1.length <= nums2.lengthとして考えれば良い
Set<Integer> set = new HashSet<>();
for (int num : nums1) {
set.add(num);
}
List<Integer> intersection = new ArrayList<>();
// 以下は同様
}
}There was a problem hiding this comment.
再帰的にintersectionを呼び出すことで
必ずnums1.length <= nums2.lengthとなり、
short_numsとlong_numsを場合分けして設定する必要がなくなるということですかね。
こんな方法があるとは考えもつかなかったです。
とてもテクニカルなので、今は適切に使いこなせるか自信がないですが、
方法の一つとして頭に入れておこうと思います。
|
|
||
| https://github.com/kt-from-j/leetcode/pull/10/commits/8aca48aa93a922d2b46e41f47ab0278ead43a93b | ||
| streamを使って短く書く方法もある。 | ||
| streamは書きなれておらず、まずはforループの練習としてstreamを使わずに書く。 |
| if (short_nums.contains(long_num)) { | ||
| intersection.add(long_num); | ||
| short_nums.remove(long_num); | ||
| } |
There was a problem hiding this comment.
ガード節というテクニックでネストを浅くできるので選択肢としてあると良いと思いました。元々のコードのネストが深くないので過剰かもしれません。
| if (short_nums.contains(long_num)) { | |
| intersection.add(long_num); | |
| short_nums.remove(long_num); | |
| } | |
| if (!short_nums.contains(long_num)) { | |
| continue; | |
| } | |
| intersection.add(long_num); | |
| short_nums.remove(long_num); |
There was a problem hiding this comment.
返信遅くなり失礼いたしました。
レビューありがとうございます。
ガード節について調べてみたのですが、処理対象外を先にはじいてから処理を行うやり方なんですね。
勉強になりました。
ネストが浅くなることで可読性やメンテナンス性が上がるなどのメリットがあるとのことで、今後コードを書くときに取り入れようと思います。
| @@ -0,0 +1,65 @@ | |||
| #STEP1 何も見ずに解く | |||
| 2重ループ(2ポインタ法)で解こうとしたが時間が切れてしまった。 | |||
There was a problem hiding this comment.
時間が切れてしまったというのは Time Limited Exceeded と判定されたということでしょうか?それともソースコードを書きあげるまでの時間が長くなりすぎてしまったということでしょうか?
There was a problem hiding this comment.
レビューありがとうございます。
ここに時間が切れてしまったと記載しているのは、後者の意味になります。
この問題に取り組んだ際、やり方を勘違いしていて、5分考えてわからなかったらそこで
STEP1が終了→STEP2に進むものだと思っており、上記のような書き方をしました。
先日コーディング練習会参加マニュアルを読み直した際、
「標準的な進め方」に、5分考えてわからなければ答えを見て正解になるところまでが第一段階とありましたので、
今取り組んでいる387ではそのように進めているところです。
| short_nums.remove(long_num); | ||
| } | ||
| } | ||
| int[] result = new int[intersection.size()]; |
There was a problem hiding this comment.
return intersection.stream()
.mapToInt(Integer::intValue)
.toArray();という書き方もできそうです。
| short_nums.add(num2); | ||
| } | ||
| long_nums = nums1; |
There was a problem hiding this comment.
参考までにスタイルガイドへのリンクを共有いたします。
https://google.github.io/styleguide/javaguide.html#s5.2.7-local-variable-names
Local variable names are written in lowerCamelCase.
なお、このスタイルガイドは“唯一の正解”というわけではなく、数あるガイドラインの一つに過ぎません。チームによって重視される書き方や慣習も異なります。そのため、ご自身の中に基準を持ちつつも、最終的にはチームの一般的な書き方に合わせることをお勧めします。
| } | ||
|
|
||
| return result; | ||
|
|
There was a problem hiding this comment.
あまり見ないとのこと、承知しました。
特に大きな意味はなく、手癖で改行をしていたのですが、
専門家から見た場合、おそらくノイズになってしまうのかもしれないと思料いたしました。
returnの後には改行をいれない方向で次回以降取り組もうと思います。
エンジニアとしての一般的な感覚がまだ身についていないので、
このような、教科書を読むだけではわからない暗黙的な常識についても身に着けていきたく思います。
(勉強不足で、暗黙的でなければ、すみません。)
今回の問題
349.Inteserction of Two Arrays
原因不明なのですがjavaのコード部分がブロックで表示されていません。
何度か試したものの直らなかったため、見にくくて申し訳ないですが、いったんプルリクエストを作成しました。
もし解決方法に心当たりがあればご教示いただきたいです。
次の問題
387. First Unique Character in a String