739. Daily Temperatures#10
Conversation
| ```java | ||
| class Solution { | ||
| public int[] dailyTemperatures(int[] temperatures) { | ||
| ArrayDeque<int[]> stack = new ArrayDeque<>(); |
There was a problem hiding this comment.
int[] の 0 番目と 1 番目にどのような値が含まれているのか、やや分かりづらいように感じました。 クラスやレコードクラスを作り、変数名やアクセサメソッド名で、どのような値が含まれているか、読み手にとって分かりやすくするとよいと思いました。
There was a problem hiding this comment.
ご確認ありがとうございます！
おっしゃる通りでしたmm
なぜかRecordクラスはEntityの定義周りでしか使わないという認識をいただいており、コーディングテストでは使えていませんでした 🙇
試しにrecordクラスで書き直したところ見通しがグッと良くなったので今後に活かしますmm
下記コミットでstep1を修正したものをstep4として実装しました。
Step4でStep1に関していただいた指摘の修正
| ArrayDeque<int[]> stack = new ArrayDeque<>(); | ||
| int[] ans = new int[temperatures.length]; | ||
|
|
||
| for (int t = 0; t < temperatures.length; t++) { |
There was a problem hiding this comment.
t という変数名が何を表しているのわかりませんでした。短いスコープで、インデックスを表す 1 文字の変数には i がよく使われると思います。また、もう少しスコープが長くなる場合は、 i ですと何を表すか分かりにくくなると思います。その場合は、 day 等、中にどのような値が含まれているかが分かる英単語・英語句にするとよいと思います。
There was a problem hiding this comment.
ありがとうございますm
temperaturesという配列名から一文字とってきてtとしていました。
自分でも実装中にtとiがごっちゃになってタイポしていたのでそこで気づくべきでした。
その場合は、 day 等、中にどのような値が含まれているかが分かる英単語・英語句にするとよいと思います。
今回は上記コメントでいただき、実装したRecordクラスのフィールド名に合わせてdayとしました。
Step4でStep1に関していただいた指摘の修正
| class Solution { | ||
| public int[] dailyTemperatures(int[] temperatures) { | ||
| int length = temperatures.length; | ||
| Deque<Integer> stack = new ArrayDeque<>(); |
There was a problem hiding this comment.
stack という変数名からは、中にどのような値が含まれているか分かりにくく感じました。 decreasingTemperatureIndices 等、中に含まれる値を想起しやすい名前を付けるとよいと思います。
There was a problem hiding this comment.
おっしゃる通りですね、、、
既存では変数名からは抽象的なデータ構造しかわからないことは課題だと思いました。
今後安易にstackなど抽象的なデータ構造を変数名とすることは避けようと思います！
| - 単にstackに積むだけではなく、stackに積む前に積むあたいがstackの一番上の値より大きいか確認する | ||
| - 大きければ、stackからpopする | ||
| - この一連の動作により単調減少のstackが作れる | ||
| - stackに積むのはindexだけでいいかなと思ったが、おそらくわかりやすさのために温度もセットで積んでいる |
There was a problem hiding this comment.
この stack 何が入っているかというと、「日付と温度の組で、その日よりも高温になった日がまだ出てきていないもの、日付の昇順、温度の降順」ですかね。コードがこのことを発見するパズルになっているという言い方もできるでしょう。どういうヒントを与えたら一番そのことに容易に気がつくでしょうか。
There was a problem hiding this comment.
どういうヒントを与えたら一番そのことに容易に気がつくでしょうか。
コード上でヒントを与えるとしたら別コメントで野田さんからいただいた指摘になるのですが、1つ目にstackの中身をRecord型で定義し日付と温度の組であることを分かりやすくすることがあると思います。
2つ目にstackという変数名を変更し、この変数の使われ方を明確化するというのがあるかと思います。
「日付と温度の組で、その日よりも高温になった日がまだ出てきていないもの、日付の昇順、温度の降順」
stackに変わる変数名の候補としては、これら全ての特性を表現するのは難しいと思いつつ下記のような候補があるかと思います。
- unresolvedTemperatureIndices
- stackに積まれているものがまだ最終的な答えを持っていないことを明確化できる
- ただ、stackが温度の降順であるという核心は伝えられない
- decreasingTemperatureIndices
- stackが温度の降順である単調スタックということが明確化される
以上から、野田さんに提案いただいたdecreasingTemperatureIndicesが最適なのかなと思いました。
ヒントを与えるなら、という視点から他者にいかにして自分の組んだパズルを理解してもらうかという思考に至り、参考になりました。
ありがとうございますmm
There was a problem hiding this comment.
私は unresolved 気に入りました。
unresolvedDays くらいで日付のみ載せるなどでもいいかもしれません。
いずれにせよ工夫や趣味で評価はぶれるものですが、パズルのヒントを(コメントも利用しながら)どう与えるか考えれば間違いはないでしょう。(管理しやすいコードにして、エンジニアリングをしやすくするという目的に沿っています。)
3 participants
https://leetcode.com/problems/daily-temperatures/description/
Given an array of integers temperatures represents the daily temperatures, return an array answer such that answer[i] is the number of days you have to wait after the ith day to get a warmer temperature. If there is no future day for which this is possible, keep answer[i] == 0 instead.
Example 1:
Constraints:
1 <= temperatures.length <= 105
30 <= temperatures[i] <= 100