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

[Core] Remove unnecessary new operators ... #1313

Merged
merged 4 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 22 additions & 12 deletions ecal/core/include/ecal/ecal_timer.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

#include <ecal/ecal_os.h>
#include <functional>
#include <memory>

namespace eCAL
{
Expand All @@ -37,13 +38,13 @@ namespace eCAL
*
* The CTimer class is used to realize simple time triggered callbacks.
**/
class ECAL_API CTimer
class CTimer
KerstinKeller marked this conversation as resolved.
Show resolved Hide resolved
{
public:
/**
* @brief Constructor.
**/
CTimer();
ECAL_API CTimer();

/**
* @brief Constructor.
Expand All @@ -52,12 +53,26 @@ namespace eCAL
* @param callback_ The callback function.
* @param delay_ Timer callback delay for first call in ms.
**/
CTimer(int timeout_, TimerCallbackT callback_, int delay_ = 0);
ECAL_API CTimer(int timeout_, TimerCallbackT callback_, int delay_ = 0);

/**
* @brief Destructor.
**/
virtual ~CTimer();
ECAL_API virtual ~CTimer();

// Object not copyable
CTimer(const CTimer&) = delete;
CTimer& operator=(const CTimer&) = delete;

/**
* @brief Move constructor
**/
ECAL_API CTimer(CTimer&& rhs);
KerstinKeller marked this conversation as resolved.
Show resolved Hide resolved

/**
* @brief Move assignment
**/
ECAL_API CTimer& operator=(CTimer&& rhs);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: move assignment operators should be marked noexcept [performance-noexcept-move-constructor]

Suggested change
ECAL_API CTimer& operator=(CTimer&& rhs);
ECAL_API CTimer& operator=(CTimer&& rhs) noexcept ;


/**
* @brief Start the timer.
Expand All @@ -68,22 +83,17 @@ namespace eCAL
*
* @return True if timer could be started.
**/
bool Start(int timeout_, TimerCallbackT callback_, int delay_ = 0);
ECAL_API bool Start(int timeout_, TimerCallbackT callback_, int delay_ = 0);

/**
* @brief Stop the timer.
*
* @return True if timer could be stopped.
**/
bool Stop();
ECAL_API bool Stop();

protected:
// class members
CTimerImpl* m_timer;

private:
// this object must not be copied.
CTimer(const CTimer&);
CTimer& operator=(const CTimer&);
std::unique_ptr<CTimerImpl> m_timer;
KerstinKeller marked this conversation as resolved.
Show resolved Hide resolved
};
}
41 changes: 25 additions & 16 deletions ecal/core/include/ecal/msg/protobuf/dynamic_json_subscriber.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#pragma once

#include <ecal/ecal.h>
#include <memory>

namespace eCAL
{
Expand All @@ -36,25 +37,38 @@ namespace eCAL
/**
* @brief eCAL dynamic protobuf to json subscriber.
**/
class ECAL_API CDynamicJSONSubscriber
class CDynamicJSONSubscriber
KerstinKeller marked this conversation as resolved.
Show resolved Hide resolved
{
public:
/**
* @brief Constructor.
**/
CDynamicJSONSubscriber();
ECAL_API CDynamicJSONSubscriber();

/**
* @brief Constructor.
*
* @param topic_name_ Unique topic name.
**/
CDynamicJSONSubscriber(const std::string& topic_name_);
ECAL_API CDynamicJSONSubscriber(const std::string& topic_name_);

/**
* @brief Destructor.
**/
~CDynamicJSONSubscriber();
ECAL_API ~CDynamicJSONSubscriber();

CDynamicJSONSubscriber(const CDynamicJSONSubscriber&) = delete;
CDynamicJSONSubscriber& operator=(const CDynamicJSONSubscriber&) = delete;

/**
* @brief Move constructor
**/
ECAL_API CDynamicJSONSubscriber(CDynamicJSONSubscriber&& rhs);
KerstinKeller marked this conversation as resolved.
Show resolved Hide resolved

/**
* @brief Move assignment
**/
ECAL_API CDynamicJSONSubscriber& operator=(CDynamicJSONSubscriber&& rhs);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: move assignment operators should be marked noexcept [performance-noexcept-move-constructor]

Suggested change
ECAL_API CDynamicJSONSubscriber& operator=(CDynamicJSONSubscriber&& rhs);
ECAL_API CDynamicJSONSubscriber& operator=(CDynamicJSONSubscriber&& rhs) noexcept ;


/**
* @brief Creates this object.
Expand All @@ -63,21 +77,21 @@ namespace eCAL
*
* @return true if it succeeds, false if it fails.
**/
void Create(const std::string& topic_name_);
ECAL_API void Create(const std::string& topic_name_);

/**
* @brief Destroys this object.
*
* @return true if it succeeds, false if it fails.
**/
void Destroy();
ECAL_API void Destroy();

/**
* @brief Query if this object is created.
*
* @return true if created, false if not.
**/
bool IsCreated() { return(created); }
ECAL_API bool IsCreated() { return(created); }
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'IsCreated' can be made const [readability-make-member-function-const]

Suggested change
ECAL_API bool IsCreated() { return(created); }
ECAL_API bool IsCreated() const { return(created); }


/**
* @brief Add callback function for incoming receives.
Expand All @@ -86,23 +100,18 @@ namespace eCAL
*
* @return True if succeeded, false if not.
**/
bool AddReceiveCallback(ReceiveCallbackT callback_);
ECAL_API bool AddReceiveCallback(ReceiveCallbackT callback_);

/**
* @brief Remove callback function for incoming receives.
*
* @return True if succeeded, false if not.
**/
bool RemReceiveCallback();
ECAL_API bool RemReceiveCallback();

protected:
bool created;
CDynamicJSONSubscriberImpl* proto_dyn_sub_impl;

private:
// this object must not be copied.
CDynamicJSONSubscriber(const CDynamicJSONSubscriber&);
CDynamicJSONSubscriber& operator=(const CDynamicJSONSubscriber&);
bool created;
std::unique_ptr<CDynamicJSONSubscriberImpl> proto_dyn_sub_impl;
};
/** @example proto_dyn_json.cpp
* This is an example how to use CDynamicJSONSubscriber to receive dynamic google::protobuf data as a JSON string with eCAL.
Expand Down
18 changes: 9 additions & 9 deletions ecal/core/include/ecal/msg/protobuf/dynamic_subscriber.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,13 @@ namespace eCAL

std::shared_ptr<google::protobuf::Message> CreateMessagePointer(const std::string& topic_name_);

bool created;
std::string topic_name;
eCAL::protobuf::CProtoDynDecoder* msg_decoder;
std::shared_ptr<google::protobuf::Message> msg_ptr;
eCAL::CSubscriber msg_sub;
ProtoMsgCallbackT msg_callback;
ProtoErrorCallbackT err_callback;
bool created;
std::string topic_name;
std::unique_ptr<eCAL::protobuf::CProtoDynDecoder> msg_decoder;
std::shared_ptr<google::protobuf::Message> msg_ptr;
eCAL::CSubscriber msg_sub;
ProtoMsgCallbackT msg_callback;
ProtoErrorCallbackT err_callback;

private:
// this object must not be copied.
Expand Down Expand Up @@ -196,7 +196,7 @@ namespace eCAL
topic_name = topic_name_;

// create message decoder
msg_decoder = new eCAL::protobuf::CProtoDynDecoder();
msg_decoder = std::make_unique<eCAL::protobuf::CProtoDynDecoder>();

// create subscriber
msg_sub.Create(topic_name_);
Expand All @@ -215,7 +215,7 @@ namespace eCAL
msg_ptr = nullptr;

// delete message decoder
delete msg_decoder;
msg_decoder.reset();

created = false;
}
Expand Down
8 changes: 2 additions & 6 deletions ecal/core/src/config/ecal_config_reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -352,14 +352,10 @@ namespace eCAL
CConfig::CConfig() :
m_impl(nullptr)
{
m_impl = new CConfigImpl();
m_impl = std::make_unique<CConfigImpl>();
}

CConfig::~CConfig()
{
delete m_impl;
m_impl = nullptr;
}
CConfig::~CConfig() = default;

void CConfig::OverwriteKeys(const std::vector<std::string>& key_vec_)
{
Expand Down
3 changes: 2 additions & 1 deletion ecal/core/src/config/ecal_config_reader.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

#include <string>
#include <vector>
#include <memory>

namespace eCAL
{
Expand All @@ -49,7 +50,7 @@ namespace eCAL
std::string get(const std::string& section_, const std::string& key_, const char* default_);

private:
CConfigImpl* m_impl;
std::unique_ptr<CConfigImpl> m_impl;
};

ECAL_API bool CfgGetBool (const std::string& section_, const std::string& key_, bool default_ = false);
Expand Down
4 changes: 2 additions & 2 deletions ecal/core/src/ecal_descgate.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ namespace eCAL
struct STopicInfoMap
{
explicit STopicInfoMap(const std::chrono::milliseconds& timeout_) :
map(new TopicInfoMap(timeout_))
map(std::make_unique<TopicInfoMap>(timeout_))
{
};
mutable std::shared_timed_mutex sync; //!< Mutex protecting the map
Expand All @@ -116,7 +116,7 @@ namespace eCAL
struct SServiceMethodInfoMap
{
explicit SServiceMethodInfoMap(const std::chrono::milliseconds& timeout_) :
map(new ServiceMethodInfoMap(timeout_))
map(std::make_unique<ServiceMethodInfoMap>(timeout_))
{
};
mutable std::shared_timed_mutex sync; //!< Mutex protecting the map
Expand Down
5 changes: 2 additions & 3 deletions ecal/core/src/monitoring/ecal_monitoring_def.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,12 @@ namespace eCAL
{
CMonitoring::CMonitoring()
{
m_monitoring_impl = new CMonitoringImpl;
m_monitoring_impl = std::make_unique<CMonitoringImpl>();
}

CMonitoring::~CMonitoring()
{
delete m_monitoring_impl;
m_monitoring_impl = nullptr;
m_monitoring_impl.reset();
}

void CMonitoring::Create()
Expand Down
2 changes: 1 addition & 1 deletion ecal/core/src/monitoring/ecal_monitoring_def.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ namespace eCAL
void GetMonitoring(eCAL::Monitoring::SMonitoring& monitoring_, unsigned int entities_ = Monitoring::Entity::All);

protected:
CMonitoringImpl* m_monitoring_impl = nullptr;
std::unique_ptr<CMonitoringImpl> m_monitoring_impl;

private:
CMonitoring(const CMonitoring&); // prevent copy-construction
Expand Down
8 changes: 4 additions & 4 deletions ecal/core/src/monitoring/ecal_monitoring_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ namespace eCAL
struct STopicMonMap
{
explicit STopicMonMap(const std::chrono::milliseconds& timeout_) :
map(new TopicMonMapT(timeout_))
map(std::make_unique<TopicMonMapT>(timeout_))
{
};
std::mutex sync;
Expand All @@ -98,7 +98,7 @@ namespace eCAL
struct SProcessMonMap
{
explicit SProcessMonMap(const std::chrono::milliseconds& timeout_) :
map(new ProcessMonMapT(timeout_))
map(std::make_unique<ProcessMonMapT>(timeout_))
{
};
std::mutex sync;
Expand All @@ -109,7 +109,7 @@ namespace eCAL
struct SServerMonMap
{
explicit SServerMonMap(const std::chrono::milliseconds& timeout_) :
map(new ServerMonMapT(timeout_))
map(std::make_unique<ServerMonMapT>(timeout_))
{
};
std::mutex sync;
Expand All @@ -120,7 +120,7 @@ namespace eCAL
struct SClientMonMap
{
explicit SClientMonMap(const std::chrono::milliseconds& timeout_) :
map(new ClientMonMapT(timeout_))
map(std::make_unique<ClientMonMapT>(timeout_))
{
};
std::mutex sync;
Expand Down
25 changes: 15 additions & 10 deletions ecal/core/src/pubsub/ecal_proto_dyn_json_sub.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ namespace eCAL
if (created) return;

// create message decoder
msg_decoder = new eCAL::protobuf::CProtoDynDecoder();
msg_decoder = std::make_unique<eCAL::protobuf::CProtoDynDecoder>();

// create subscriber
msg_sub.Create(topic_name_);
Expand All @@ -101,7 +101,7 @@ namespace eCAL
msg_sub.Destroy();

// delete message decoder
delete msg_decoder;
msg_decoder.reset();

created = false;
}
Expand Down Expand Up @@ -173,11 +173,11 @@ namespace eCAL
}
}

bool created;
eCAL::protobuf::CProtoDynDecoder* msg_decoder;
std::string msg_string;
eCAL::CSubscriber msg_sub;
ReceiveCallbackT msg_callback;
bool created;
std::unique_ptr<eCAL::protobuf::CProtoDynDecoder> msg_decoder;
std::string msg_string;
eCAL::CSubscriber msg_sub;
ReceiveCallbackT msg_callback;

std::string topic_type;
std::string topic_type_full;
Expand Down Expand Up @@ -209,10 +209,16 @@ namespace eCAL
Destroy();
}

CDynamicJSONSubscriber::CDynamicJSONSubscriber(CDynamicJSONSubscriber&& rhs)
KerstinKeller marked this conversation as resolved.
Show resolved Hide resolved
= default;

CDynamicJSONSubscriber& CDynamicJSONSubscriber::operator=(CDynamicJSONSubscriber&& rhs)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: move assignment operators should be marked noexcept [performance-noexcept-move-constructor]

ecal/core/src/pubsub/ecal_proto_dyn_json_sub.cpp:215:

-       = default;
+  noexcept       = default;

= default;

void CDynamicJSONSubscriber::Create(const std::string& topic_name_)
{
if (created) return;
proto_dyn_sub_impl = new CDynamicJSONSubscriberImpl(topic_name_);
proto_dyn_sub_impl = std::make_unique<CDynamicJSONSubscriberImpl>(topic_name_);
proto_dyn_sub_impl->Create(topic_name_);
created = true;
}
Expand All @@ -221,8 +227,7 @@ namespace eCAL
{
if (!created) return;
proto_dyn_sub_impl->Destroy();
delete proto_dyn_sub_impl;
proto_dyn_sub_impl = nullptr;
proto_dyn_sub_impl.reset();
created = false;
}

Expand Down
Loading
Loading