From 7929cfde95dfdb17337a70d61f4ffbef51575110 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 14 Mar 2026 03:16:12 +0000 Subject: [PATCH 1/2] Initial plan From 90454cf5790d302593d249da79cc6d32b8eb6a09 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 14 Mar 2026 03:26:27 +0000 Subject: [PATCH 2/2] Fix safety, correctness, and C++ best practice issues across codebase Co-authored-by: urbytes21 <118428350+urbytes21@users.noreply.github.com> --- src/controller/pid/pid.cpp | 21 +++----- src/controller/pid/pid.h | 10 ++-- src/core/concurrency/SharingData.cpp | 8 +-- src/excercise/account/Account.cpp | 4 +- src/excercise/student_manager/Manager.cpp | 57 +++++++--------------- src/excercise/student_manager/Manager.h | 5 +- src/excercise/student_manager/Student.cpp | 5 +- src/leetcode/arrays/two_sum/TwoSum.cpp | 12 ++--- src/main.cpp | 4 +- src/patterns/behavioral/ChainOfCommand.cpp | 1 + src/patterns/creational/Singleton.cpp | 22 ++++----- src/patterns/structural/Facade.cpp | 8 +-- src/socket/simple_tcp/TCPServer.h | 7 +-- 13 files changed, 64 insertions(+), 100 deletions(-) diff --git a/src/controller/pid/pid.cpp b/src/controller/pid/pid.cpp index 0548ef8..0864ef8 100644 --- a/src/controller/pid/pid.cpp +++ b/src/controller/pid/pid.cpp @@ -20,15 +20,11 @@ * THE SOFTWARE. */ -#ifndef _PID_SOURCE_ -#define _PID_SOURCE_ - -#include #include +#include +#include #include "pid.h" -using namespace std; - class PIDImpl { public: @@ -49,17 +45,14 @@ class PIDImpl PID::PID( double dt, double max, double min, double Kp, double Kd, double Ki ) + : pimpl_(std::make_unique(dt, max, min, Kp, Kd, Ki)) { - pimpl = new PIDImpl(dt,max,min,Kp,Kd,Ki); } double PID::calculate( double setpoint, double pv ) { - return pimpl->calculate(setpoint,pv); -} -PID::~PID() -{ - delete pimpl; + return pimpl_->calculate(setpoint,pv); } +PID::~PID() = default; /** * Implementation @@ -110,6 +103,4 @@ double PIDImpl::calculate( double setpoint, double pv ) PIDImpl::~PIDImpl() { -} - -#endif \ No newline at end of file +} \ No newline at end of file diff --git a/src/controller/pid/pid.h b/src/controller/pid/pid.h index d9a2445..31fe5df 100644 --- a/src/controller/pid/pid.h +++ b/src/controller/pid/pid.h @@ -20,8 +20,8 @@ * THE SOFTWARE. */ -#ifndef _PID_H_ -#define _PID_H_ +#pragma once +#include class PIDImpl; class PID { @@ -43,7 +43,5 @@ class PID { PID& operator=(const PID& other) = delete; private: - PIDImpl* pimpl; -}; - -#endif \ No newline at end of file + std::unique_ptr pimpl_; +}; \ No newline at end of file diff --git a/src/core/concurrency/SharingData.cpp b/src/core/concurrency/SharingData.cpp index 6026b30..4defdf1 100644 --- a/src/core/concurrency/SharingData.cpp +++ b/src/core/concurrency/SharingData.cpp @@ -35,13 +35,13 @@ void run() { class SharingData : public IExample { - std::string group() const { return "core/concurrency"; } - std::string name() const { return "SharingData"; } - std::string description() const { + std::string group() const override { return "core/concurrency"; } + std::string name() const override { return "SharingData"; } + std::string description() const override { return "The examples for sharing data"; } - void execute() { run(); } + void execute() override { run(); } }; REGISTER_EXAMPLE(SharingData, "core/concurrency", "SharingData"); \ No newline at end of file diff --git a/src/excercise/account/Account.cpp b/src/excercise/account/Account.cpp index 762b2ad..e23e066 100644 --- a/src/excercise/account/Account.cpp +++ b/src/excercise/account/Account.cpp @@ -21,7 +21,7 @@ void Account::credit(double amount) { balance_ += amount; std::ostringstream oss; - std::time_t now = time(NULL); + std::time_t now = time(nullptr); oss << std::fixed << std::setprecision(4) << "[credit] " << amount << " at " << time2Str(now); log_.push_back(oss.str()); @@ -36,7 +36,7 @@ void Account::debit(double amount) { balance_ -= debit_amount; std::ostringstream oss; - std::time_t now = time(NULL); + std::time_t now = time(nullptr); oss << std::fixed << std::setprecision(4) << "[debit] " << debit_amount << " at " << time2Str(now); log_.push_back(oss.str()); diff --git a/src/excercise/student_manager/Manager.cpp b/src/excercise/student_manager/Manager.cpp index 7e03f4f..7fb0752 100644 --- a/src/excercise/student_manager/Manager.cpp +++ b/src/excercise/student_manager/Manager.cpp @@ -3,38 +3,26 @@ #include #include -Manager::~Manager() { - for (auto it : students_) { - delete it; - } -} - void Manager::add() { std::string name = inputStr("Name?", "Invalid"); std::string address = inputStr("Address?", "Invalid"); - Student* s = new Student(name, address); - students_.push_back(s); + students_.push_back(std::make_unique(name, address)); sortByName(); std::cout << "Add successful!\n"; } void Manager::remove() { std::string code = inputStr("Code?", "Invalid"); - // for (auto it = students_.begin(); it != students_.end(); ++it) { - // if ((*it)->getCode().substr(0, code.size()) == code) { - // delete *it; - // students_.erase(it); - // std::cout << "Remove successfull!\n"; - // break; - // } - // } auto it = std::find_if( students_.begin(), students_.end(), - [&code](const Student* s) { return s && s->getCode().find(code) == 0; }); + [&code](const std::unique_ptr& s) { + return s && s->getCode().find(code) == 0; + }); if (it != students_.end()) { students_.erase(it); - std::cout << "Remove successfull!\n"; + std::cout << "Remove successful!\n"; + return; } std::cout << "Student not found!\n"; @@ -67,25 +55,21 @@ void Manager::update() { } std::ostream& operator<<(std::ostream& os, const Manager& manager) { - for (const auto* const s : manager.students_) { + for (const auto& s : manager.students_) { os << *s << "\n"; } return os; } Student* Manager::findByCode(const std::string& cod) { - // for (auto it : students_) { - // // if (it->getCode().substr(0, cod.size()) == cod) // partial match - // if (it->getCode() == cod) { - // return it; - // } - // } const auto it = std::find_if( students_.begin(), students_.end(), - [&cod](const Student* s) { return s && s->getCode() == cod; }); + [&cod](const std::unique_ptr& s) { + return s && s->getCode() == cod; + }); if (it != students_.end()) { - return *it; + return it->get(); } std::cout << "Student not found !!\n"; @@ -94,7 +78,8 @@ Student* Manager::findByCode(const std::string& cod) { void Manager::sortByName() { std::sort(students_.begin(), students_.end(), - [](const Student* a, const Student* b) { return *a < *b; }); + [](const std::unique_ptr& a, + const std::unique_ptr& b) { return *a < *b; }); } std::string Manager::trim(const std::string& str) { @@ -111,17 +96,11 @@ std::string Manager::trim(const std::string& str) { std::vector Manager::findByName() { std::string name = inputStr("Name?", "Invalid Name?"); std::vector result; - // for (auto it : students_) { - // // contain - // if (it->getName().find(name) != std::string::npos) { - // result.push_back(it); - // } - // } - std::copy_if(students_.begin(), students_.end(), std::back_inserter(result), - [&name](const Student* const s) { - return s && s->getName().find(name) != std::string::npos; - }); - + for (const auto& s : students_) { + if (s && s->getName().find(name) != std::string::npos) { + result.push_back(s.get()); + } + } return result; } diff --git a/src/excercise/student_manager/Manager.h b/src/excercise/student_manager/Manager.h index 258fd6c..b8888df 100644 --- a/src/excercise/student_manager/Manager.h +++ b/src/excercise/student_manager/Manager.h @@ -1,10 +1,9 @@ #pragma once +#include #include #include "Student.h" class Manager { public: - ~Manager(); - void add(); void update(); void remove(); @@ -19,5 +18,5 @@ class Manager { friend std::ostream& operator<<(std::ostream& os, const Manager& manager); private: - std::vector students_; + std::vector> students_; }; \ No newline at end of file diff --git a/src/excercise/student_manager/Student.cpp b/src/excercise/student_manager/Student.cpp index abbaa3e..8bbcf59 100644 --- a/src/excercise/student_manager/Student.cpp +++ b/src/excercise/student_manager/Student.cpp @@ -20,14 +20,13 @@ Student::Student(const Student& other) void Student::setCode() { // get name code XXX std::string nameCode = name_; - - name_.append(3, ' '); // so, name code at least 3 charactor + nameCode.append(3, ' '); // pad nameCode to at least 3 characters nameCode.resize(3); std::replace(nameCode.begin(), nameCode.end(), ' ', 'X'); std::transform(nameCode.begin(), nameCode.end(), nameCode.begin(), ::toupper); // get time code XXXX - time_t now = time(NULL); + time_t now = time(nullptr); std::string timeCode = std::to_string(now); timeCode = timeCode.substr(timeCode.size() - 3, timeCode.size()); code_ = nameCode + timeCode; diff --git a/src/leetcode/arrays/two_sum/TwoSum.cpp b/src/leetcode/arrays/two_sum/TwoSum.cpp index b6c73c3..cf7fb4e 100644 --- a/src/leetcode/arrays/two_sum/TwoSum.cpp +++ b/src/leetcode/arrays/two_sum/TwoSum.cpp @@ -6,17 +6,17 @@ * Space complexity: O(1) */ std::vector Solution::twoSum(const std::vector& nums, int target) { - for (int i = 0; i < nums.size(); ++i) { - int diff = target - nums.at(i); - for (int j = i + 1; j < nums.size(); ++j) { - if (diff == nums.at(j)) { - return std::vector{i, j}; + for (std::size_t i = 0; i < nums.size(); ++i) { + int diff = target - nums[i]; + for (std::size_t j = i + 1; j < nums.size(); ++j) { + if (diff == nums[j]) { + return {static_cast(i), static_cast(j)}; } } } // no solution - return std::vector{-1, -1}; + return {-1, -1}; } // #include diff --git a/src/main.cpp b/src/main.cpp index ff9783b..9c779c1 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -55,7 +55,7 @@ void runMenu() { break; } - if (gChoice < 1 || gChoice > groups.size()) { + if (gChoice < 1 || static_cast(gChoice) > groups.size()) { std::cout << "Invalid group choice\n"; continue; } @@ -102,7 +102,7 @@ void runMenu() { } } -int main(int argc, char* argv[]) { +int main([[maybe_unused]] int argc, [[maybe_unused]] char* argv[]) { std::cout << std::endl; if (__cplusplus == 202302L) std::cout << "C++23"; diff --git a/src/patterns/behavioral/ChainOfCommand.cpp b/src/patterns/behavioral/ChainOfCommand.cpp index fca6de1..fa0b9fe 100644 --- a/src/patterns/behavioral/ChainOfCommand.cpp +++ b/src/patterns/behavioral/ChainOfCommand.cpp @@ -20,6 +20,7 @@ namespace CoR { */ class IHandler { public: + virtual ~IHandler() = default; virtual void setNextHandler(IHandler* handler) = 0; virtual IHandler* setNext(IHandler* handler) = 0; virtual void handle(const std::string& request) = 0; diff --git a/src/patterns/creational/Singleton.cpp b/src/patterns/creational/Singleton.cpp index 65e845e..afd71cc 100644 --- a/src/patterns/creational/Singleton.cpp +++ b/src/patterns/creational/Singleton.cpp @@ -22,8 +22,6 @@ namespace SingletonPattern { */ class Singleton { private: - static inline Singleton* instance = nullptr; - static inline int num = 0; /** * The Singleton's constructor should always be private to prevent direct * construction calls with the `new` operator. @@ -37,31 +35,29 @@ class Singleton { // 2. Should not be assignable Singleton& operator=(const Singleton& other) = delete; - static Singleton* getInstance() { - if (instance == nullptr) { - instance = new Singleton(); - num++; - } - + static Singleton& getInstance() { + // Thread-safe since C++11: static local initialization is guaranteed + // to happen exactly once even in a multi-threaded environment. + static Singleton instance; return instance; } void operation() const { - std::cout << "Singleton operating num:" << num << "\n"; + std::cout << "Singleton operating\n"; } }; namespace Client { -void clientCode(const Singleton* const s) { - s->operation(); +void clientCode(const Singleton& s) { + s.operation(); } } // namespace Client void run() { - const Singleton* s1 = Singleton::getInstance(); + Singleton& s1 = Singleton::getInstance(); Client::clientCode(s1); - const Singleton* s2 = Singleton::getInstance(); + Singleton& s2 = Singleton::getInstance(); Client::clientCode(s2); // Singleton* s3 = new Singleton(); // ERROR diff --git a/src/patterns/structural/Facade.cpp b/src/patterns/structural/Facade.cpp index 04f8940..6ee2d2d 100644 --- a/src/patterns/structural/Facade.cpp +++ b/src/patterns/structural/Facade.cpp @@ -136,10 +136,10 @@ class RequestFacade { const ValidatorSubSystem* s2 = nullptr, const LoggerSubSystem* s3 = nullptr, const BackendSubSystem* s4 = nullptr) { - this->auth_ = s1 ?: new AuthSubSystem; - this->validator_ = s2 ?: new ValidatorSubSystem; - this->logger_ = s3 ?: new LoggerSubSystem; - this->backend_ = s4 ?: new BackendSubSystem; + this->auth_ = s1 ? s1 : new AuthSubSystem; + this->validator_ = s2 ? s2 : new ValidatorSubSystem; + this->logger_ = s3 ? s3 : new LoggerSubSystem; + this->backend_ = s4 ? s4 : new BackendSubSystem; } ~RequestFacade() { diff --git a/src/socket/simple_tcp/TCPServer.h b/src/socket/simple_tcp/TCPServer.h index 2f8c107..0e1acf6 100644 --- a/src/socket/simple_tcp/TCPServer.h +++ b/src/socket/simple_tcp/TCPServer.h @@ -1,4 +1,5 @@ #pragma once +#include #include #include #include @@ -51,7 +52,7 @@ class TCPServer { void setServerFD(int fd); private: - uint16_t port_; // TCP ports are in 0 - 65535 - int server_fd_{-1}; // socket descriptor - volatile bool running_{false}; + uint16_t port_; // TCP ports are in 0 - 65535 + int server_fd_{-1}; // socket descriptor + std::atomic running_{false}; }; \ No newline at end of file