refactor the refactor

Author: andiwand

CMakeLists.txt

@@ -87,6 +87,8 @@ set(ODR_SOURCE_FILES
         "src/odr/odr.cpp"
         "src/odr/quantity.cpp"
         "src/odr/style.cpp"
+        "src/odr/table_dimension.cpp"
+        "src/odr/table_position.cpp"
 
         "${CMAKE_CURRENT_BINARY_DIR}/src/odr/internal/git_info.cpp"
         "src/odr/internal/magic.cpp"
@@ -106,7 +108,7 @@ set(ODR_SOURCE_FILES
         "src/odr/internal/common/random.cpp"
         "src/odr/internal/common/style.cpp"
         "src/odr/internal/common/table_cursor.cpp"
-        "src/odr/internal/common/table_position.cpp"
+        "src/odr/table_position.cpp"
         "src/odr/internal/common/table_range.cpp"
         "src/odr/internal/common/temporary_file.cpp"
 

src/odr/definitions.hpp

@@ -0,0 +1,11 @@
+#pragma once
+
+#include <cstdint>
+
+namespace odr {
+
+using ElementIdentifier = std::uint64_t;
+
+static constexpr ElementIdentifier null_element_id{0};
+
+} // namespace odr

src/odr/document_element.cpp

@@ -2,6 +2,8 @@
 
 #include <odr/file.hpp>
 #include <odr/style.hpp>
+#include <odr/table_dimension.hpp>
+#include <odr/table_position.hpp>
 
 #include <odr/internal/abstract/document_element.hpp>
 
@@ -10,13 +12,13 @@ namespace odr {
 Element::Element() = default;
 
 Element::Element(const internal::abstract::ElementAdapter *adapter,
-                 const ExtendedElementIdentifier identifier)
+                 const ElementIdentifier identifier)
     : m_adapter{adapter}, m_identifier{identifier} {}
 
 Element::operator bool() const { return exists_(); }
 
 bool Element::exists_() const {
-  return m_adapter != nullptr && !m_identifier.is_null();
+  return m_adapter != nullptr && m_identifier != null_element_id;
 }
 
 ElementType Element::type() const {
@@ -66,6 +68,10 @@ Page Element::as_page() const {
   return {m_adapter, m_identifier, m_adapter->page_adapter(m_identifier)};
 }
 
+SheetCell Element::as_sheet_cell() const {
+  return {m_adapter, m_identifier, m_adapter->sheet_cell_adapter(m_identifier)};
+}
+
 MasterPage Element::as_master_page() const {
   return {m_adapter, m_identifier,
           m_adapter->master_page_adapter(m_identifier)};
@@ -152,7 +158,7 @@ ElementIterator::ElementIterator() = default;
 
 ElementIterator::ElementIterator(
     const internal::abstract::ElementAdapter *adapter,
-    const ExtendedElementIdentifier identifier)
+    const ElementIdentifier identifier)
     : m_adapter{adapter}, m_identifier{identifier} {}
 
 ElementIterator::reference ElementIterator::operator*() const {
@@ -174,7 +180,7 @@ ElementIterator ElementIterator::operator++(int) {
 }
 
 bool ElementIterator::exists_() const {
-  return m_adapter != nullptr && !m_identifier.is_null();
+  return m_adapter != nullptr && m_identifier != null_element_id;
 }
 
 ElementRange::ElementRange() = default;
@@ -198,7 +204,7 @@ MasterPage TextRoot::first_master_page() const {
   if (!exists_()) {
     return {};
   }
-  const ExtendedElementIdentifier master_page_id =
+  const ElementIdentifier master_page_id =
       m_adapter2->text_root_first_master_page(m_identifier);
   return {m_adapter, master_page_id,
           m_adapter->master_page_adapter(master_page_id)};
@@ -216,7 +222,7 @@ MasterPage Slide::master_page() const {
   if (!exists_()) {
     return {};
   }
-  const ExtendedElementIdentifier master_page_id =
+  const ElementIdentifier master_page_id =
       m_adapter2->slide_master_page(m_identifier);
   return {m_adapter, master_page_id,
           m_adapter->master_page_adapter(master_page_id)};
@@ -237,30 +243,12 @@ Sheet::content(const std::optional<TableDimensions> range) const {
                    : TableDimensions();
 }
 
-SheetColumn Sheet::column(const std::uint32_t column) const {
-  if (!exists_()) {
-    return {};
-  }
-  const ExtendedElementIdentifier column_id =
-      m_adapter2->sheet_column(m_identifier, column);
-  return {m_adapter, column_id, m_adapter->sheet_column_adapter(column_id)};
-}
-
-SheetRow Sheet::row(const std::uint32_t row) const {
-  if (!exists_()) {
-    return {};
-  }
-  const ExtendedElementIdentifier row_id =
-      m_adapter2->sheet_row(m_identifier, row);
-  return {m_adapter, row_id, m_adapter->sheet_row_adapter(row_id)};
-}
-
 SheetCell Sheet::cell(const std::uint32_t column,
                       const std::uint32_t row) const {
   if (!exists_()) {
     return {};
   }
-  const ExtendedElementIdentifier cell_id =
+  const ElementIdentifier cell_id =
       m_adapter2->sheet_cell(m_identifier, column, row);
   return {m_adapter, cell_id, m_adapter->sheet_cell_adapter(cell_id)};
 }
@@ -269,27 +257,28 @@ ElementRange Sheet::shapes() const {
   if (!exists_()) {
     return {};
   }
-  const ExtendedElementIdentifier first_shape_id =
+  const ElementIdentifier first_shape_id =
       m_adapter2->sheet_first_shape(m_identifier);
   return ElementRange(ElementIterator(m_adapter, first_shape_id));
 }
 
-TableColumnStyle SheetColumn::style() const {
-  return exists_() ? m_adapter2->sheet_column_style(m_identifier)
-                   : TableColumnStyle();
+TableStyle Sheet::style() const {
+  return exists_() ? m_adapter2->sheet_style(m_identifier) : TableStyle();
 }
 
-TableRowStyle SheetRow::style() const {
-  return exists_() ? m_adapter2->sheet_row_style(m_identifier)
-                   : TableRowStyle();
+TableColumnStyle Sheet::column_style(const std::uint32_t column) const {
+  return exists_() ? m_adapter2->sheet_column_style(m_identifier, column)
+                   : TableColumnStyle();
 }
 
-std::uint32_t SheetCell::column() const {
-  return exists_() ? m_adapter2->sheet_cell_column(m_identifier) : 0;
+TableRowStyle Sheet::row_style(const std::uint32_t row) const {
+  return exists_() ? m_adapter2->sheet_row_style(m_identifier, row)
+                   : TableRowStyle();
 }
 
-std::uint32_t SheetCell::row() const {
-  return exists_() ? m_adapter2->sheet_cell_row(m_identifier) : 0;
+TablePosition SheetCell::position() const {
+  return exists_() ? m_adapter2->sheet_cell_position(m_identifier)
+                   : TablePosition(0, 0);
 }
 
 bool SheetCell::is_covered() const {
@@ -323,7 +312,7 @@ MasterPage Page::master_page() const {
   if (!exists_()) {
     return {};
   }
-  const ExtendedElementIdentifier master_page_id =
+  const ElementIdentifier master_page_id =
       m_adapter2->page_master_page(m_identifier);
   return {m_adapter, master_page_id,
           m_adapter->master_page_adapter(master_page_id)};
@@ -383,16 +372,15 @@ TableRow Table::first_row() const {
   if (!exists_()) {
     return {};
   }
-  const ExtendedElementIdentifier row_id =
-      m_adapter2->table_first_row(m_identifier);
+  const ElementIdentifier row_id = m_adapter2->table_first_row(m_identifier);
   return {m_adapter, row_id, m_adapter->table_row_adapter(row_id)};
 }
 
 TableColumn Table::first_column() const {
   if (!exists_()) {
     return {};
   }
-  const ExtendedElementIdentifier column_id =
+  const ElementIdentifier column_id =
       m_adapter2->table_first_column(m_identifier);
   return {m_adapter, column_id, m_adapter->table_column_adapter(column_id)};
 }

src/odr/document_element.hpp

@@ -4,7 +4,21 @@
 #include <optional>
 #include <string>
 
-#include <odr/document_element_identifier.hpp>
+#include <odr/definitions.hpp>
+
+namespace odr {
+class TablePosition;
+struct TableDimensions;
+class File;
+struct TextStyle;
+struct ParagraphStyle;
+struct TableStyle;
+struct TableColumnStyle;
+struct TableRowStyle;
+struct TableCellStyle;
+struct GraphicStyle;
+struct PageLayout;
+} // namespace odr
 
 namespace odr::internal::abstract {
 class Document;
@@ -39,17 +53,6 @@ class ImageAdapter;
 
 namespace odr {
 
-class File;
-struct TextStyle;
-struct ParagraphStyle;
-struct TableStyle;
-struct TableColumnStyle;
-struct TableRowStyle;
-struct TableCellStyle;
-struct GraphicStyle;
-struct PageLayout;
-struct TableDimensions;
-
 class Element;
 class ElementIterator;
 class ElementRange;
@@ -90,8 +93,6 @@ enum class ElementType {
 
   master_page,
 
-  sheet_column,
-  sheet_row,
   sheet_cell,
 
   text,
@@ -141,7 +142,7 @@ class Element {
 public:
   Element();
   Element(const internal::abstract::ElementAdapter *adapter,
-          ExtendedElementIdentifier identifier);
+          ElementIdentifier identifier);
 
   explicit operator bool() const;
 
@@ -160,6 +161,7 @@ class Element {
   [[nodiscard]] Slide as_slide() const;
   [[nodiscard]] Sheet as_sheet() const;
   [[nodiscard]] Page as_page() const;
+  [[nodiscard]] SheetCell as_sheet_cell() const;
   [[nodiscard]] MasterPage as_master_page() const;
   [[nodiscard]] LineBreak as_line_break() const;
   [[nodiscard]] Paragraph as_paragraph() const;
@@ -181,7 +183,7 @@ class Element {
 
 protected:
   const internal::abstract::ElementAdapter *m_adapter{nullptr};
-  ExtendedElementIdentifier m_identifier;
+  ElementIdentifier m_identifier;
 
   friend bool operator==(const Element &lhs, const Element &rhs) {
     return lhs.m_identifier == rhs.m_identifier;
@@ -201,7 +203,7 @@ class ElementIterator {
 
   ElementIterator();
   ElementIterator(const internal::abstract::ElementAdapter *adapter,
-                  ExtendedElementIdentifier identifier);
+                  ElementIdentifier identifier);
 
   reference operator*() const;
 
@@ -210,7 +212,7 @@ class ElementIterator {
 
 private:
   const internal::abstract::ElementAdapter *m_adapter{nullptr};
-  ExtendedElementIdentifier m_identifier;
+  ElementIdentifier m_identifier{null_element_id};
 
   friend bool operator==(const ElementIterator &lhs,
                          const ElementIterator &rhs) {
@@ -240,7 +242,7 @@ template <typename T> class ElementBase : public Element {
 public:
   ElementBase() = default;
   ElementBase(const internal::abstract::ElementAdapter *adapter,
-              const ExtendedElementIdentifier identifier, const T *adapter2)
+              const ElementIdentifier identifier, const T *adapter2)
       : Element(adapter, identifier), m_adapter2{adapter2} {}
 
   explicit operator bool() const { return exists_(); }
@@ -286,28 +288,12 @@ class Sheet final : public ElementBase<internal::abstract::SheetAdapter> {
   [[nodiscard]] TableDimensions
   content(std::optional<TableDimensions> range) const;
 
-  [[nodiscard]] SheetColumn column(std::uint32_t column) const;
-  [[nodiscard]] SheetRow row(std::uint32_t row) const;
   [[nodiscard]] SheetCell cell(std::uint32_t column, std::uint32_t row) const;
-
   [[nodiscard]] ElementRange shapes() const;
-};
-
-/// @brief Represents a sheet column element in a document.
-class SheetColumn final
-    : public ElementBase<internal::abstract::SheetColumnAdapter> {
-public:
-  using ElementBase::ElementBase;
 
-  [[nodiscard]] TableColumnStyle style() const;
-};
-
-/// @brief Represents a sheet row element in a document.
-class SheetRow final : public ElementBase<internal::abstract::SheetRowAdapter> {
-public:
-  using ElementBase::ElementBase;
-
-  [[nodiscard]] TableRowStyle style() const;
+  [[nodiscard]] TableStyle style() const;
+  [[nodiscard]] TableColumnStyle column_style(std::uint32_t column) const;
+  [[nodiscard]] TableRowStyle row_style(std::uint32_t row) const;
 };
 
 /// @brief Represents a sheet cell element in a document.
@@ -316,8 +302,7 @@ class SheetCell final
 public:
   using ElementBase::ElementBase;
 
-  [[nodiscard]] std::uint32_t column() const;
-  [[nodiscard]] std::uint32_t row() const;
+  [[nodiscard]] TablePosition position() const;
   [[nodiscard]] bool is_covered() const;
   [[nodiscard]] TableDimensions span() const;
   [[nodiscard]] ValueType value_type() const;