BewareMyPower commented on code in PR #139:
URL: https://github.com/apache/pulsar-client-cpp/pull/139#discussion_r1065663539
##########
include/pulsar/ProducerConfiguration.h:
##########
@@ -532,11 +532,15 @@ class PULSAR_PUBLIC ProducerConfiguration {
*/
ProducerAccessMode getAccessMode() const;
- friend class PulsarWrapper;
-
private:
- struct Impl;
- std::shared_ptr<ProducerConfigurationImpl> impl_;
+ typedef std::shared_ptr<ProducerConfigurationImpl>
ProducerConfigurationImplPtr;
+ ProducerConfigurationImplPtr impl_;
+
+ ProducerConfiguration(ProducerConfigurationImplPtr& impl);
Review Comment:
Pass a `std::shared_ptr` by const reference or value, don't pass it by
non-const reference. Actually, I found this constructor is never used. And
could you avoid other unrelated changes (`typedef`, moving the position of
`friend`) as well?
```diff
diff --git a/include/pulsar/ProducerConfiguration.h
b/include/pulsar/ProducerConfiguration.h
index dc708e9..ca11bd0 100644
--- a/include/pulsar/ProducerConfiguration.h
+++ b/include/pulsar/ProducerConfiguration.h
@@ -532,15 +532,12 @@ class PULSAR_PUBLIC ProducerConfiguration {
*/
ProducerAccessMode getAccessMode() const;
- private:
- typedef std::shared_ptr<ProducerConfigurationImpl>
ProducerConfigurationImplPtr;
- ProducerConfigurationImplPtr impl_;
-
- ProducerConfiguration(ProducerConfigurationImplPtr& impl);
-
friend class PulsarWrapper;
- friend class ConsumerImpl;
friend class ProducerImpl;
+ friend class ConsumerImpl;
+
+ private:
+ std::shared_ptr<ProducerConfigurationImpl> impl_;
};
} // namespace pulsar
#endif /* PULSAR_PRODUCERCONFIGURATION_H_ */
diff --git a/lib/ProducerConfiguration.cc b/lib/ProducerConfiguration.cc
index 3c1bd24..0ee79f6 100644
--- a/lib/ProducerConfiguration.cc
+++ b/lib/ProducerConfiguration.cc
@@ -32,8 +32,6 @@ ProducerConfiguration::~ProducerConfiguration() {}
ProducerConfiguration::ProducerConfiguration(const ProducerConfiguration&
x) : impl_(x.impl_) {}
-ProducerConfiguration::ProducerConfiguration(ProducerConfigurationImplPtr&
impl) : impl_(impl) {}
-
ProducerConfiguration& ProducerConfiguration::operator=(const
ProducerConfiguration& x) {
impl_ = x.impl_;
return *this;
```
--
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]
