cleanup more (#466)

Author: andiwand

src/odr/document.cpp

@@ -12,7 +12,7 @@ namespace odr {
 
 Document::Document(std::shared_ptr<internal::abstract::Document> impl)
     : m_impl{std::move(impl)} {
-  if (!m_impl) {
+  if (m_impl == nullptr) {
     throw NullPointerError("document is null");
   }
 }

src/odr/document_element.hpp

@@ -7,7 +7,7 @@
 #include <odr/definitions.hpp>
 
 namespace odr {
-class TablePosition;
+struct TablePosition;
 struct TableDimensions;
 class DocumentPath;
 class File;
@@ -29,8 +29,6 @@ class TextRootAdapter;
 class SlideAdapter;
 class PageAdapter;
 class SheetAdapter;
-class SheetColumnAdapter;
-class SheetRowAdapter;
 class SheetCellAdapter;
 class MasterPageAdapter;
 class LineBreakAdapter;
@@ -60,8 +58,6 @@ class ElementRange;
 class TextRoot;
 class Slide;
 class Sheet;
-class SheetColumn;
-class SheetRow;
 class SheetCell;
 class Page;
 class MasterPage;

src/odr/file.cpp

@@ -167,40 +167,45 @@ bool DecodedFile::is_pdf_file() const {
 }
 
 TextFile DecodedFile::as_text_file() const {
-  if (const auto text_file =
-          std::dynamic_pointer_cast<internal::abstract::TextFile>(m_impl)) {
+  if (const std::shared_ptr text_file =
+          std::dynamic_pointer_cast<internal::abstract::TextFile>(m_impl);
+      text_file != nullptr) {
     return TextFile(text_file);
   }
   throw NoTextFile();
 }
 
 ImageFile DecodedFile::as_image_file() const {
-  if (const auto image_file =
-          std::dynamic_pointer_cast<internal::abstract::ImageFile>(m_impl)) {
+  if (const std::shared_ptr image_file =
+          std::dynamic_pointer_cast<internal::abstract::ImageFile>(m_impl);
+      image_file != nullptr) {
     return ImageFile(image_file);
   }
   throw NoImageFile();
 }
 
 ArchiveFile DecodedFile::as_archive_file() const {
-  if (const auto archive_file =
-          std::dynamic_pointer_cast<internal::abstract::ArchiveFile>(m_impl)) {
+  if (const std::shared_ptr archive_file =
+          std::dynamic_pointer_cast<internal::abstract::ArchiveFile>(m_impl);
+      archive_file != nullptr) {
     return ArchiveFile(archive_file);
   }
   throw NoArchiveFile();
 }
 
 DocumentFile DecodedFile::as_document_file() const {
-  if (const auto document_file =
-          std::dynamic_pointer_cast<internal::abstract::DocumentFile>(m_impl)) {
+  if (const std::shared_ptr document_file =
+          std::dynamic_pointer_cast<internal::abstract::DocumentFile>(m_impl);
+      document_file != nullptr) {
     return DocumentFile(document_file);
   }
   throw NoDocumentFile();
 }
 
 PdfFile DecodedFile::as_pdf_file() const {
-  if (const auto pdf_file =
-          std::dynamic_pointer_cast<internal::abstract::PdfFile>(m_impl)) {
+  if (const std::shared_ptr pdf_file =
+          std::dynamic_pointer_cast<internal::abstract::PdfFile>(m_impl);
+      pdf_file != nullptr) {
     return PdfFile(pdf_file);
   }
   throw NoPdfFile();

src/odr/filesystem.cpp

@@ -22,7 +22,7 @@ FileWalker::FileWalker(FileWalker &&other) noexcept = default;
 FileWalker::~FileWalker() = default;
 
 FileWalker &FileWalker::operator=(const FileWalker &other) {
-  if (this == &other) {
+  if (&other == this) {
     return *this;
   }
   m_impl = other.m_impl->clone();

src/odr/html.cpp

@@ -296,7 +296,8 @@ HtmlService html::translate(const DocumentFile &document_file,
 #ifdef ODR_WITH_WVWARE
   if (const std::shared_ptr wv_document_file =
           std::dynamic_pointer_cast<WvWareLegacyMicrosoftFile>(
-              document_file_impl)) {
+              document_file_impl);
+      wv_document_file != nullptr) {
     std::filesystem::create_directories(cache_path);
     return internal::html::create_wvware_oldms_service(
         *wv_document_file, cache_path, config, std::move(logger));
@@ -315,7 +316,8 @@ HtmlService html::translate(const PdfFile &pdf_file,
 
 #ifdef ODR_WITH_PDF2HTMLEX
   if (const std::shared_ptr poppler_pdf_file =
-          std::dynamic_pointer_cast<PopplerPdfFile>(pdf_file_impl)) {
+          std::dynamic_pointer_cast<PopplerPdfFile>(pdf_file_impl);
+      poppler_pdf_file != nullptr) {
     std::filesystem::create_directories(cache_path);
     return internal::html::create_poppler_pdf_service(
         *poppler_pdf_file, cache_path, config, std::move(logger));

src/odr/http_server.cpp

@@ -2,14 +2,12 @@
 
 #include <odr/exceptions.hpp>
 #include <odr/file.hpp>
-#include <odr/filesystem.hpp>
 #include <odr/html.hpp>
-#include <odr/internal/pdf_poppler/poppler_pdf_file.hpp>
 
 #include <httplib/httplib.h>
 
 #include <atomic>
-#include <memory>
+#include <filesystem>
 #include <sstream>
 
 namespace odr {
@@ -23,9 +21,9 @@ class HttpServer::Impl {
     // This prevents crashes when exceptions occur during request processing.
     m_server->set_exception_handler([this](const httplib::Request & /*req*/,
                                            httplib::Response &res,
-                                           std::exception_ptr ep) {
+                                           const std::exception_ptr &ep) {
       try {
-        if (ep) {
+        if (ep != nullptr) {
           std::rethrow_exception(ep);
         }
       } catch (const std::exception &e) {
@@ -73,7 +71,7 @@ class HttpServer::Impl {
     // here (via unique_ptr::reset), we ensure all threads are joined
     // before any other members are destroyed.
     m_stopping.store(true, std::memory_order_release);
-    if (m_server) {
+    if (m_server != nullptr) {
       m_server->stop();
       m_server.reset(); // Destroy server, join all thread pool threads
     }
@@ -160,7 +158,7 @@ class HttpServer::Impl {
     m_content.emplace(prefix, Content{prefix, std::move(service)});
   }
 
-  void listen(const std::string &host, const std::uint32_t port) {
+  void listen(const std::string &host, const std::uint32_t port) const {
     ODR_VERBOSE(*m_logger, "Listening on " << host << ":" << port);
 
     m_server->listen(host, static_cast<int>(port));
@@ -186,7 +184,7 @@ class HttpServer::Impl {
     // This prevents new requests from starting while we're shutting down.
     m_stopping.store(true, std::memory_order_release);
 
-    if (m_server) {
+    if (m_server != nullptr) {
       // Stop the server to prevent new connections.
       // Note: httplib::Server::stop() signals shutdown but thread pool
       // threads may still be running. They only fully stop when the

src/odr/internal/abstract/document.hpp

@@ -13,7 +13,7 @@ enum class ValueType;
 enum class AnchorType;
 struct PageLayout;
 struct TableDimensions;
-class TablePosition;
+struct TablePosition;
 struct TableStyle;
 struct TableColumnStyle;
 struct TableRowStyle;

src/odr/internal/cfb/cfb_util.cpp

@@ -133,13 +133,13 @@ std::optional<Archive::Entry> Archive::Entry::child() const {
 }
 
 void Archive::Iterator::dig_left_() {
-  if (!m_entry) {
+  if (!m_entry.has_value()) {
     return;
   }
 
   while (true) {
     const std::optional<Entry> left = m_entry->left();
-    if (!left) {
+    if (!left.has_value()) {
       break;
     }
     m_ancestors.push_back(*m_entry);
@@ -148,11 +148,11 @@ void Archive::Iterator::dig_left_() {
 }
 
 void Archive::Iterator::next_() {
-  if (!m_entry) {
+  if (!m_entry.has_value()) {
     return;
   }
 
-  if (const std::optional<Entry> child = m_entry->child()) {
+  if (const std::optional<Entry> child = m_entry->child(); child.has_value()) {
     m_directories.push_back(*m_entry);
     m_entry = child;
     dig_left_();
@@ -163,11 +163,11 @@ void Archive::Iterator::next_() {
 }
 
 void Archive::Iterator::next_flat_() {
-  if (!m_entry) {
+  if (!m_entry.has_value()) {
     return;
   }
 
-  if (const std::optional<Entry> right = m_entry->right()) {
+  if (const std::optional<Entry> right = m_entry->right(); right.has_value()) {
     m_entry = right;
     dig_left_();
     return;

src/odr/internal/common/path.hpp

@@ -94,9 +94,9 @@ class Path {
 
 private:
   std::string m_path;
-  std::uint32_t m_upwards;
-  std::uint32_t m_downwards;
-  bool m_absolute;
+  std::uint32_t m_upwards{0};
+  std::uint32_t m_downwards{0};
+  bool m_absolute{false};
 
   friend std::ostream &operator<<(std::ostream &os, const Path &p) {
     return os << p.m_path;

src/odr/internal/common/table_range.cpp

@@ -28,9 +28,8 @@ std::string TableRange::to_string() const noexcept {
 }
 
 bool TableRange::contains(const TablePosition &position) const noexcept {
-  return m_from.column() <= position.column() &&
-         m_to.column() > position.column() && m_from.row() <= position.row() &&
-         m_to.row() > position.row();
+  return m_from.column <= position.column && m_to.column > position.column &&
+         m_from.row <= position.row && m_to.row > position.row;
 }
 
 } // namespace odr::internal