Skip to content

Conversation

@Fuminiton
Copy link
Owner

loaded = 0 # reset
loaded += w

return task_duration_days <= days
Copy link

Choose a reason for hiding this comment

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

task_duration_daysがdaysを超えたときにreturn Falseしてもよいですね

Copy link
Owner Author

Choose a reason for hiding this comment

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

レビューありがとうございます。

これ気付けてなかったです。
weights.lengthのサイズが大きい場合などは必須で入れたいですね

maybe_first_can_ship_capacity = mid + 1

return maybe_first_can_ship_capacity
```
Copy link

Choose a reason for hiding this comment

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

FFF?????TTTTがあって、maybe_first_can_ship_capacityは最初の?で、can_ship_capacityは最初のTでしょうか。
それならばcan_ship_capacityを返したほうが直観的に思いました。

それと変数名的にmaybe_first_can_ship_capacityが帰ってくるのは怖くなりました。
仕事を任せた人が「たぶん最小はこれです」と渡してきたら不安になるような感じでしょうか。

Copy link
Owner Author

Choose a reason for hiding this comment

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

maybe_first_can_ship_capacityは最初の?で、can_ship_capacityは最初のTでしょうか。

maybe_first_can_ship_capacityが最初のTか、Fで、can_ship_capacityが常にTになる値のイメージです。

ループの打ち切りがmaybe_first_can_ship_capacity==can_ship_capacityなので伝わるだろうと思いmaybeの方を返すようにしましたが、おっしゃる通りmaybeな値が返ってくるのは良くないですね

Copy link

@fuga-98 fuga-98 Jun 4, 2025

Choose a reason for hiding this comment

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

ループの打ち切りがmaybe_first_can_ship_capacity==can_ship_capacityなので伝わるだろう

これは伝わると思います。


return processing_days <= days

maybe_min_capacity = max(weights)
Copy link

Choose a reason for hiding this comment

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

upper_limit,lower_limitとかでしょうか

Copy link
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます。
limitが適切な表現ですね


return processing_days <= days

maybe_first_can_ship_capacity = max(weights)
Copy link

Choose a reason for hiding this comment

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

maybe_first_can_ship_capacity という変数名からは「おそらく最初(の船?)は積載量を積載可能だろう」というニュアンスに感じました。表現したい内容とずれているように思います。 maybe_min_shippable_capacity はいかがでしょうか?これもやや微妙ではありますが…。

Copy link
Owner Author

Choose a reason for hiding this comment

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

レビューありがとうございます。
maybe_min_shippable_capacityの方が適切ですね。

left, rightより良い変数がないかはもう少し考えるべきでした。

### 感想
- daysが0以下は早期リターンすべきだった
- 変数名が冗長すぎるかもしれない
- rangeでリストが作られると思っていたが、実際に作られるのはstartとstop地点、ステップと長さを持ったrange objectらしい
Copy link

Choose a reason for hiding this comment

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

The advantage of the range type over a regular list or tuple is that a range object will always take the same (small) amount of memory, no matter the size of the range it represents (as it only stores the start, stop and step values, calculating individual items and subranges as needed).
https://docs.python.org/3.13/library/stdtypes.html#typesseq-range

Copy link
Owner Author

Choose a reason for hiding this comment

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

リンクの共有ありがとうございます。
根拠のurlもつけるべきでした。


return task_duration_days <= days

min_capacity_candidates = range(sum(weights))
Copy link

Choose a reason for hiding this comment

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

答えとしてあり得る最大値は sum(weights) なので range(sum(weights+1)) の方がいいと思いました。

Copy link
Owner Author

Choose a reason for hiding this comment

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

レビューありがとうございます。
おっしゃる通りですね。

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.

6 participants