Skip to content

929.unique email addresses#14

Open
nicah4o wants to merge 1 commit into
mainfrom
929.unique-email-addresses
Open

929.unique email addresses#14
nicah4o wants to merge 1 commit into
mainfrom
929.unique-email-addresses

Conversation

@nicah4o
Copy link
Copy Markdown
Owner

@nicah4o nicah4o commented May 16, 2026

 2.'+'のみ先に出現した場合(local)
 3.'@'のみ出現した場合(domain)
 4.'+'と'@'がどちらも出現した場合(domain)
で場合分けした。3と4の場合はdomain部なので何もせずに文字列を書き写せばよい。
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か)でコード中のif分も分かれていた方が読みやすいと思いました

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.

for文を分ける方が状態変数が減るので良いかもしれません。

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.

@liquo-rice
レビューありがとうございます。
なるほど、'@'の前後でforを二回に分けることで'@'のフラグは消えますね。

at = true;
}
if ((at == false && email[i] == '.') ||
(at == false && plus == true)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ここにもう一段インデントを入れると良いと思います。

if (email[i] == '@') {
at = true;
}
if ((at == false && email[i] == '.') ||
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

at == false という書き方は、あまり見ないように思います。 !at と書くことが多いと思います。

public:
int numUniqueEmails(vector<string>& emails) {
vector<string> canonicalized_emails = Canonicalize(emails);
set<string> unique_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.

set<string> unique_emails(canonicalized_emails.begin(), canonicalized_emails.end());

のほうがシンプルだと思いました。

sort(canonicalized_emails.begin(), canonicalized_emails.end());
canonicalized_emails.erase(unique(canonicalized_emails.begin(), canonicalized_emails.end()), canonicalized_emails.end());
return canonicalized_emails.size();

もありだと思います。

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.

ありがとうございます。
この方法ならset使わずメモリ節約できますね。


private:
vector<string> Canonicalize(const vector<string>& emails) {
auto CanonicalizeEmail = [this](const string& email) {
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++ では、 inner function の用途でラムダ式を書くのは、あまり見ないかもしれません。イベントハンドラー等に使うのはよく見かけます。

public:
int numUniqueEmails(vector<string>& emails) {
unordered_set<string> email_set;
for (string email : 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.

こちらのコメントをご参照ください。
5ky7/arai60#22 (comment)

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.

レビューありがとうございます。
string型はこの場合、間接参照にしたほうがよいのですね。判断基準参考になります。

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.

4 participants