Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MultiPolygonにarea()とperimeter()を追加(#1185) #1187

Merged
merged 3 commits into from
Jan 18, 2024

Conversation

Ogame3334
Copy link
Contributor

MultiPolygonにnoexceptなconstメンバ変数であるarea()とperimeter()を実装

Copy link
Member

@Reputeless Reputeless left a comment

Choose a reason for hiding this comment

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

2 箇所の修正をお願いします。

double MultiPolygon::area() const noexcept
{
double total = 0;
for (const auto& polygon : *this)
Copy link
Member

Choose a reason for hiding this comment

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

わずかな変更でもう一歩だけ改善できます。

range-based for は https://cpprefjp.github.io/lang/cpp17/generalizing_the_range-based_for_loop.html のように展開されます。ここで *this を使うと begin-exprMultiPolygon::begin() になります。すると

  • MultiPolygon::begin()MultiPolygon::base_type::begin()std::vector<Polygon>::begin()

のような呼び出しになります。

一方で m_data を使うと

  • MultiPolygon::base_type::begin()std::vector<Polygon>::begin()

のように 1 つ減らせます。

登場するすべての begin() はインライン関数なので、最適化が有効であれば最終的に同じコードになって差は生じませんが、Debug ビルドなどで最適化を無効にしている場合は少しだけコストが嵩むことになります。修正をお願いします。

double MultiPolygon::perimeter() const noexcept
{
double total = 0;
for (const auto& polygon : *this)
Copy link
Member

Choose a reason for hiding this comment

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

.area() のコメントと同じです。

@Ogame3334
Copy link
Contributor Author

フィードバックありがとうございます!
他の実装(MultiPolygon& MultiPolygon::scaleAt(const Vec2 pos, const Vec2 s)など)を参考にし、そちらでは*thisを用いていたのでそちらにしたのですが、他の部分の実装とどう異なるのでしょうか?

@Reputeless
Copy link
Member

Reputeless commented Jan 18, 2024

ご指摘のとおりです。他の箇所も m_data を使うよう修正すべきです。
他の箇所を修正するコミットも、本件の完了後に別途、あるいは今回の pull-request に含めてもらえると大きな貢献になります。

@Reputeless Reputeless merged commit 0e0b5fc into Siv3D:v6_develop Jan 18, 2024
2 checks passed
@Reputeless
Copy link
Member

Merged. Thanks!


◆ 初めて Siv3D にコミットした方へのご案内

コミッタの方の名前を AUTHORS に記載します。
表示する名前をお知らせください。特に希望が無い場合は GitHub プロフィールの ID が使われます。

◆ Organization への招待について

OpenSiv3D 本体および Siv3D ドキュメントのリポジトリにコミットをした方、その他顕著な貢献をされた方には、GitHub の Siv3D Organization メンバー への招待が送られます。
招待を受諾し、上記ページで自身のメンバー参加表示設定を「Public」に変更すると、GitHub の自身のプロフィールページに Siv3D のアイコンが表示されます。「Private」のままだと、自身と他のメンバーにしかアイコンは表示されません。

@Ogame3334
Copy link
Contributor Author

ありがとうございます!
表示する名前は「緑獺おがめ」でお願いします!

Reputeless added a commit that referenced this pull request Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants