Skip to content

Create Unique Email Addresses.md#16

Open
irohafternoon wants to merge 1 commit intomainfrom
929.-Unique-Email-Addresses
Open

Create Unique Email Addresses.md#16
irohafternoon wants to merge 1 commit intomainfrom
929.-Unique-Email-Addresses

Conversation

@irohafternoon
Copy link
Copy Markdown
Owner

- 文字列の一部を置換
- std::erase https://cpprefjp.github.io/reference/deque/deque/erase_free.html (c++20から)

範囲や要素の指定は様々なので、使っていったりリファレンスを眺めて覚えていこう
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

c++にも色々な文字列操作の関数が用意されているんですね。
勉強になります。

// stringを返り値にした時のコピーを防ぐため、出力用の空文字列を参照して更新する
static void canonicalizeEmail(const std::string& email, std::string& blank_for_canonicalized_email) {
auto at_position = email.rfind('@');
auto local = email.substr(0, at_position);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

好みの範囲かもしれませんが、localusernameなどの方が直感的な気がします。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

自分は、localとdomainに分かれるというemailのルールに則っているのでこれでいい気がします

}
return formatted_address;
}
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

分かりやすかったです。
これも好みの範囲かもしれず恐縮ですが、is_ignoreis_waiting_atなどの方が頭を使わずに理解できるような気がします。

public:
int numUniqueEmails(std::vector<std::string>& emails) {
std::set<std::string> mailing_list;
for (const auto& adress : emails) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

細かいところですが、"address"ですね

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.

ご指摘ありがとうございます!
スペルミスには気を付けます

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

フランス語は、adresse なので英語はどこかで増えたんですね。

local = local.substr(0, plus_position);
std::erase(local, '.');
auto domain = email.substr(at_position);
blank_for_canonicalized_email += local + domain;
Copy link
Copy Markdown

@t0hsumi t0hsumi Apr 10, 2025

Choose a reason for hiding this comment

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

ここは=ではダメなんでしょうか?

呼び出し側がblank_for_canonicalized_emailを空文字列以外で呼び出したときを考えて少し不安になりました。

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.

ご指摘ありがとうございます
おっしゃる通りで、空文字以外が入力された時も考えて、=にするほうが確実に良いと気づきました

auto plus_position = local.find_first_of('+');
local = local.substr(0, plus_position);
std::erase(local, '.');
auto domain = email.substr(at_position);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

好みですが、自分ならL164の下にこれ入れるかなと思います。
「localとdomainに分ける→localをいじる→localとdomainをくっつける」が自然な流れな気がしています。

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.

コメントありがとうございます
実際どちらも考えました。
domainが登場して何もないまま間が空くことを嫌って今の形を選択したのですが、どちらも好むという方がいるのが知れてよかったです

- 部分文字列を取得
- replace https://cpprefjp.github.io/reference/string/basic_string/replace.html
- 文字列の一部を置換
- std::erase https://cpprefjp.github.io/reference/deque/deque/erase_free.html (c++20から)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

よく使う erase は下のリンクではと思ったんですが、この PR で使っているのは std::erase ですね。
https://cpprefjp.github.io/reference/string/basic_string/erase.html
( C++03 から)

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.

ありがとうございます、貼っていただいたリンクはイテレータの範囲で削除するものと理解しました。
std::eraseは要素の値を直接指定して削除できるので便利だと感じました。
c++20など、新しいバージョンには便利な機能がほかにもありそうなので、キャッチアップしていこうと思います。

- replace https://cpprefjp.github.io/reference/string/basic_string/replace.html
- 文字列の一部を置換
- std::erase https://cpprefjp.github.io/reference/deque/deque/erase_free.html (c++20から)

Copy link
Copy Markdown

@oda oda Apr 10, 2025

Choose a reason for hiding this comment

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

#include <regex>
もみておいてほしいかもしれません。

Copy link
Copy Markdown
Owner Author

@irohafternoon irohafternoon Apr 11, 2025

Choose a reason for hiding this comment

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

ありがとうございます、そもそもこのような文字列含むstringの操作には#include <string>が必要ということに気が付いていませんでした。また、std::eraseには #include <algorithm> が必要でした。
また正規表現を使う際には regex が必要のようでした。
新しいメソッドを調べる際には、何をincludeするべきかにも気を付けます

public:
int numUniqueEmails(std::vector<std::string>& emails) {
std::set<std::string> mailing_list;
for (const auto& adress : emails) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

フランス語は、adresse なので英語はどこかで増えたんですね。

return unique_emails.size();
}
private:
// stringを返り値にした時のコピーを防ぐため、出力用の空文字列を参照して更新する
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

私は古い世代の人間なので、参照を使ってインプレースで更新する形を好むのですが、現代的には、RVO と move などで最適化がより効くようになったので、単に return にしてしまうのはよく見ます。

今回の場合、特に、最後で必ず local + domain で文字列が一回構築されるし、用法からしても比較的速度がクリティカルではないコードではないのかなと推察します。このあたりは正解があるものではないですが、いろいろな要素を比較して「パレート最適」にはしておき、あとは好みで選ぶところでしょう。

パレート最適:
https://discord.com/channels/1084280443945353267/1237649827240742942/1359594009168973844

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.

ありがとうございます。
まずはいろいろな手法の引き出しを身に着け、また何かを犠牲にせずとも改善できないかを考えながら、「パレート最適」に近づけるようにしたいです。そのうえで「パレート最適」のなかで、シチュエーションに応じた選択をできることを目標にします。

昔習ったパレート最適や生産可能性フロンティアを思い出して懐かしくなりました

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@irohafternoon
参考にですが、move等は「Effective Modern C++」に記載されております。この会で時々推薦されている本になります🙇

return mailing_list.size();
}
private:
static std::string formatAddress(const std::string& adress) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

参考にですが、GoogleGuideでは関数名をアッパーキャメルとしております🙇
https://google.github.io/styleguide/cppguide.html#Function_Names

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