Copilot commented on code in PR #3129:
URL: https://github.com/apache/brpc/pull/3129#discussion_r2463260568
##########
src/bvar/multi_dimension.h:
##########
@@ -162,11 +175,8 @@ class MultiDimension : public MVariable<KeyType> {
template <typename K>
static typename std::enable_if<!butil::is_same<K, key_type>::value>::type
insert_metrics_map(MetricMap& bg, const K& labels_value, op_value_type
metric) {
- key_type labels_value_str;
- for (auto& label : labels_value) {
- // key_type::value_type must be able to convert to std::string.
- labels_value_str.push_back(std::string(label));
- }
+ // key_type::value_type must be able to convert to std::string.
+ key_type labels_value_str(labels_value.cbegin(), labels_value.cend());
Review Comment:
This range-based constructor assumes KeyType supports construction from
iterators, which may not be true for all container types (e.g., std::array).
The previous loop-based approach was safer for heterogeneous types. Consider
retaining the loop or adding a static_assert to verify iterator-based
construction is supported.
```suggestion
key_type labels_value_str;
for (auto it = labels_value.cbegin(); it != labels_value.cend();
++it) {
labels_value_str.push_back(*it);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
