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

about Registry::Add some question #671

Closed
Marsmzhao-TX opened this issue Dec 16, 2023 · 2 comments · Fixed by #673
Closed

about Registry::Add some question #671

Marsmzhao-TX opened this issue Dec 16, 2023 · 2 comments · Fixed by #673

Comments

@Marsmzhao-TX
Copy link

Marsmzhao-TX commented Dec 16, 2023

I noticed that inside the function template <typename T> Family<T>& Registry::Add(const std::string& name , const std::string& help ,const Labels& labels), the "same_name_and_labels" is returned using 'name' and 'labels' as keys.

However, later in the function, there is another duplication check using 'name' only as the key:

auto it = std::find_if(families.begin(), families.end(), same_name);
if (it != families.end()) {
  throw std::invalid_argument("Family name already exists");
}

I am considering whether the latter check can be removed to make the key of Family consist of both 'name' and 'labels'. Could this adjustment help better optimize the function?

gjasny added a commit that referenced this issue Dec 18, 2023
@gjasny
Copy link
Collaborator

gjasny commented Dec 18, 2023

Hello,

you're right. In former times we had a third merge strategy. When it got removed, we did not optimize the code.
Could you please take a look at #673?

Thanks,
Gregor

gjasny added a commit that referenced this issue Dec 19, 2023
gjasny added a commit that referenced this issue Dec 19, 2023
@Marsmzhao-TX
Copy link
Author

o( ̄▽ ̄)ブ

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 a pull request may close this issue.

2 participants