Fix safety, correctness, and modern C++ best practices across codebase

src/controller/pid/pid.cpp

@@ -20,15 +20,11 @@
  * THE SOFTWARE.
  */
 
-#ifndef _PID_SOURCE_
-#define _PID_SOURCE_
-
-#include <iostream>
 #include <cmath>
+#include <iostream>
+#include <memory>
 #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<PIDImpl>(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
+}

src/controller/pid/pid.h

@@ -20,8 +20,8 @@
  * THE SOFTWARE.
  */
 
-#ifndef _PID_H_
-#define _PID_H_
+#pragma once
+#include <memory>
 
 class PIDImpl;
 class PID {
@@ -43,7 +43,5 @@ class PID {
     PID& operator=(const PID& other) = delete;
 
  private:
-  PIDImpl* pimpl;
-};
-
-#endif
+  std::unique_ptr<PIDImpl> pimpl_;
+};

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 <thread> sharing data";
   }
 
-  void execute() { run(); }
+  void execute() override { run(); }
 };
 
 REGISTER_EXAMPLE(SharingData, "core/concurrency", "SharingData");

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());

src/excercise/student_manager/Manager.cpp

@@ -3,38 +3,26 @@
 #include <sstream>
 #include <stdexcept>
 
-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<Student>(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<Student>& 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<Student>& 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<Student>& a,
+               const std::unique_ptr<Student>& 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<Student*> Manager::findByName() {
   std::string name = inputStr("Name?", "Invalid Name?");
   std::vector<Student*> 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;
 }
 

src/excercise/student_manager/Manager.h

@@ -1,10 +1,9 @@
 #pragma once
+#include <memory>
 #include <vector>
 #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<Student*> students_;
+  std::vector<std::unique_ptr<Student>> students_;
 };

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;

src/leetcode/arrays/two_sum/TwoSum.cpp

@@ -6,17 +6,17 @@
  * Space complexity: O(1)
  */
 std::vector<int> Solution::twoSum(const std::vector<int>& 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<int>{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<int>(i), static_cast<int>(j)};
       }
     }
   }
 
   // no solution
-  return std::vector<int>{-1, -1};
+  return {-1, -1};
 }
 
 // #include <unordered_map>

src/main.cpp

@@ -55,7 +55,7 @@ void runMenu() {
       break;
     }
 
-    if (gChoice < 1 || gChoice > groups.size()) {
+    if (gChoice < 1 || static_cast<std::size_t>(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";

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;

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

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() {

src/socket/simple_tcp/TCPServer.h

@@ -1,4 +1,5 @@
 #pragma once
+#include <atomic>
 #include <stdint.h>
 #include <stdexcept>
 #include <string>
@@ -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<bool> running_{false};
 };