Skip to content

Conversation

erutako
Copy link
Owner

@erutako erutako commented Jun 4, 2024

@erutako erutako changed the title Two sum (from Grind 75 questions) Two sum (from Grind 75 questions) Easy Jun 4, 2024
@erutako erutako changed the title Two sum (from Grind 75 questions) Easy Two Sum (from Grind 75 questions) Easy Jun 5, 2024
Copy link

@fhiyo fhiyo left a comment

Choose a reason for hiding this comment

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

Java見るのかなり久しぶりなので、変なこと言ってたらすみません

5分26秒

### 感想(他の方のコードなどを読んで)
- HashTableよりHashMapのほうが1ms早くなった。このあたりも仕様を把握して使い分けできるようにしたい。
Copy link

Choose a reason for hiding this comment

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

もしleetcodeで出力されたRuntimeの値を見てるんでしたら、結構ブレが大きいので大きな差でなければ気にしないでいいかもです

Copy link
Owner Author

Choose a reason for hiding this comment

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

確かにおっしゃるとおりですねmm
細かい差はあまり気にしないようにしますmm

```Java
class Solution {
public int[] twoSum(int[] nums, int target) {
HashMap<Integer, Integer> map = new HashMap<>();
Copy link

Choose a reason for hiding this comment

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

ここは Map<Integer, Integer> map = new HashMap<>(); のように広いインターフェースを使ってもいいかもです。

参考: https://stackoverflow.com/questions/1348199/what-is-the-difference-between-the-hashmap-and-map-objects-in-java

Copy link
Owner Author

Choose a reason for hiding this comment

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

特別な理由がない限り、広いinterfaceで定義したほうが良さそうですね!
ありがとうございますmm

課題の言語化でだいぶコードのイメージがついた。
とりあえず書いてみる。
ここまでで11分。
今回は値が見付からなかったときは`[-1, -1]`で返すことにする。
Copy link

Choose a reason for hiding this comment

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

Javaならnullを返すのも一つかもですね

Copy link
Owner Author

@erutako erutako Jun 5, 2024

Choose a reason for hiding this comment

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

確かにそうですね!
nullだと呼び出し側はどんな印象を受けるのでしょうか。
個人的には戻り値が配列のシグネチャならnullは返ってこないことを期待したい(Optionalで返してほしい、nullチェックを呼び出し側にさせるのは嫌だなという感覚)ですが、[-1, -1]で返しても結局呼び出し側で値が見つかったかどうかのチェックするので、一緒ですね...
(独り言ですmm)

Copy link

Choose a reason for hiding this comment

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

個人的には戻り値が配列のシグネチャならnullは返ってこないことを期待したい(Optionalで返してほしい、nullチェックを呼び出し側にさせるのは嫌だなという感覚)ですが、[-1, -1]で返しても結局呼び出し側で値が見つかったかどうかのチェックする

うーむ難しいですよね...正直、よく分からないです。入力が前提条件を満たさないときに特殊な値を返すとして、nullが返るのと[-1, -1]が返るのどちらがびっくりしないかという話かなと思っていて、Javaだったら前者だろうか...?と思ってコメントした感じでした。Optionalで返したいの分かります。

Copy link
Owner Author

Choose a reason for hiding this comment

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

びっくりしないかという捉え方すごくわかりやすいです!
そういう観点で戻りを考えることもしていこうと思いましたmm
ありがとうございます!

@erutako erutako changed the title Two Sum (from Grind 75 questions) Easy 1. Two Sum (from Grind 75 questions) Easy Jun 5, 2024
@erutako
Copy link
Owner Author

erutako commented Jun 5, 2024

@fhiyo san
レビューありがとうございます!mm


### 感想(他の方のコードなどを読んで)
- 今回は必ず解が見つかる条件だったが、見つからないときにどのような値を返すかは考慮してよかった
- 謎にbreakで抜けてからreturnしているが、普通にその場でreturnで良かった
Copy link

Choose a reason for hiding this comment

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

これは違和感が結構ありますね。

Copy link
Owner Author

Choose a reason for hiding this comment

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

そうですよねmm
普段あまりしないのですが、がむしゃらに考えてコード書いていたら最終的によくわからないコードが誕生していましたmm
自分もかなり違和感を持つコードだったので、すぐに気付けるようにトレーニングしますmm

@erutako erutako merged commit 1a6eac3 into main Jun 10, 2024
@erutako erutako changed the title 1. Two Sum (from Grind 75 questions) Easy 1. Two Sum (from Grind 75 questions Easy-Array) Jun 10, 2024
@erutako erutako deleted the twoSum branch July 20, 2024 11:29
@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.

3 participants