Skip to content

1. Two Sum#3

Merged
colorbox merged 2 commits intomainfrom
1
Mar 11, 2024
Merged

1. Two Sum#3
colorbox merged 2 commits intomainfrom
1

Conversation

@colorbox
Copy link
Copy Markdown
Owner

@colorbox colorbox commented Mar 10, 2024

Copy link
Copy Markdown

@hayashi-ay hayashi-ay left a comment

Choose a reason for hiding this comment

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

良いと思います。

int currentOther, index;

for(index = 0; index < nums.size(); index++){
currentOther = target - nums[index];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

もっといい命名がある気がします。「現在の他」とはどういうことだろうと思いました。paircomplementnumToFindあたりでしょうか?もっと良いのがあるかもしれないですが。

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.

回答となる組を作るイメージで、nums[i]をcurrentでその片方という意味でcurrentOtherとしてましたが
numToFindのほうがわかりやすいですね

for(index = 0; index < nums.size(); index++){
currentOther = target - nums[index];
if(numMap.find(currentOther) != numMap.end()){
break;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

(小田さんのコメントと重複しますが)ここでreturnしてあげれば、int currentOther, indexのスコープをfor文に限定できます。これくらいのコードであれば、変数の数も少ないのでスコープを限定しなくても書けちゃいますが、ある程度複雑になると不必要に脳内のワーキングメモリを使うことになります。

numMap[nums[index]] = index;
}

return { index, numMap[currentOther] };
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

(これも小田さんのコメントと重複しますが)課題制約で解があることは保証されていますが、解がない場合も考慮してみるとどうでしょうか。

@colorbox colorbox merged commit ed2dbff into main Mar 11, 2024
@colorbox colorbox deleted the 1 branch March 11, 2024 16:49
@rihib rihib mentioned this pull request Aug 6, 2024
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.

2 participants