Conversation
| if (2 * x == target && it->second.size() >= 2) { | ||
| retVal.push_back(it->second[0]); | ||
| retVal.push_back(it->second[1]); | ||
| return retVal; | ||
| } else if (2 * x != target) { |
There was a problem hiding this comment.
2 * x と target の関係を2回書いていますが、
if (2 * x == target) {
if (it->second.size() >= 2) {
...
}
} else {
...
}という気持ちではないですかね。
There was a problem hiding this comment.
コメントありがとうございます!その気持ちでした。
There was a problem hiding this comment.
if (2 * x == target && it->second.size() >= 2) {
...
return retVal;
}
if (2 * x != target) {
...
}こちらもわりと自然な気持ちとは思います。考え方次第です。いくつかの自明な変形を常に頭においておきましょう。
There was a problem hiding this comment.
承知しました!常にいくつかの選択肢から取捨選択できるようにしたいと思います。
|
|
||
| ### step3 | ||
|
|
||
| unordered_mapにvectorを入れなくていいシンプルな方法をChatGPTに提示されたのでそれで書き直して、3回正解するまでやり直した。 No newline at end of file |
There was a problem hiding this comment.
There was a problem hiding this comment.
承知しました!まずは手作業でやれることを意識して考えるようにします。
|
(補足ですが) class Solution {
public:
vector<int> twoSum(const vector<int>& nums, int target) {
if (nums.size() < 2) {
return {};
} |
|
コメントありがとうございます!たしかにそうですね。解が存在しない場合にどうするかも考えるようにしてみます。 |
| class Solution { | ||
| public: | ||
| vector<int> twoSum(vector<int>& nums, int target) { | ||
| unordered_map<int, vector<int>> map; |
There was a problem hiding this comment.
型名をそのまま変数名に含めても、読み手にとって得られる情報はあまり増えないように感じました。
変数名には、どのような値が格納されているかが分かる名前を付けると、より読みやすくなると思います。特に map や unordered_map の場合は、キーと値にどのような値が含まれているかを変数名で示すとよいと思います。自分はよく (キー)_to_(値) という命名をします。今回の場合は num_to_indices はいかがでしょうか?
There was a problem hiding this comment.
コメントありがとうございます!num_to_indicesのような名前をつけるよう心がけます。
| for (int i = 0; i < nums.size(); i++) { | ||
| map[nums[i]].push_back(i); | ||
| } | ||
| vector<int> retVal; |
There was a problem hiding this comment.
一般に変数のスコープは短ければ短いほど良いと思います。これは、変数のスコープを短くすることによって、読み手の短期記憶の容量を節約することができる多mです。
retVal は、以下の for 文の中でしか使っていないため、 for 文の中、特に使用する直前で定義するとよいと思います。
There was a problem hiding this comment.
これはおっしゃる通りですね。変数のスコープも意識するようにしてできるだけ短くするようにします。
| for (int i = 0; i < nums.size(); i++) { | ||
| map[nums[i]].push_back(i); | ||
| } | ||
| vector<int> retVal; |
There was a problem hiding this comment.
retVal という変数にはあまり情報がないように思いました。自分なら indices や、冗長に書くなら indices_of_two_numbers と付けると思います。
TakayaShirai
left a comment
There was a problem hiding this comment.
全体的な感想として、すでに指摘があるように変数の命名に情報があまりないように感じ、そこで少し読みづらさを感じました。手作業でやることをまず意識して、手作業での役割を変数名としてそのまま用いる(例:数字をインデックスに変換する表 → num_to_indicies )などをすると、個人的にわかりやすい変数名を考えやすいと思います。
|
コメントありがとうございます。何か情報のある変数名を付けるよう心がけます。 |
この問題:https://leetcode.com/problems/two-sum/
次に解く問題:https://leetcode.com/problems/linked-list-cycle/description/