From 8ba37872e960f123cfc5ef68bafb1af613e29067 Mon Sep 17 00:00:00 2001 From: Arda Cinar Date: Sun, 11 Dec 2022 22:45:11 +0300 Subject: [PATCH] SpaceAnalyzer: Remove an unnecessary level of inheritance The TreeMapNode and TreeMap structs inside TreeMapWidget.h both had single implementers, TreeNode and Tree inside main.cpp. The indirection was removed and the new structures were moved to their own file --- .../Applications/SpaceAnalyzer/CMakeLists.txt | 1 + Userland/Applications/SpaceAnalyzer/Tree.cpp | 16 +++++++ Userland/Applications/SpaceAnalyzer/Tree.h | 45 +++++++++++++++++++ .../SpaceAnalyzer/TreeMapWidget.cpp | 26 +++++------ .../SpaceAnalyzer/TreeMapWidget.h | 27 +++-------- Userland/Applications/SpaceAnalyzer/main.cpp | 43 ++---------------- 6 files changed, 84 insertions(+), 74 deletions(-) create mode 100644 Userland/Applications/SpaceAnalyzer/Tree.cpp create mode 100644 Userland/Applications/SpaceAnalyzer/Tree.h diff --git a/Userland/Applications/SpaceAnalyzer/CMakeLists.txt b/Userland/Applications/SpaceAnalyzer/CMakeLists.txt index 0d6dbbab2a..28beaa4ca6 100644 --- a/Userland/Applications/SpaceAnalyzer/CMakeLists.txt +++ b/Userland/Applications/SpaceAnalyzer/CMakeLists.txt @@ -7,6 +7,7 @@ compile_gml(SpaceAnalyzer.gml SpaceAnalyzerGML.h space_analyzer_gml) set(SOURCES TreeMapWidget.cpp + Tree.cpp main.cpp ) diff --git a/Userland/Applications/SpaceAnalyzer/Tree.cpp b/Userland/Applications/SpaceAnalyzer/Tree.cpp new file mode 100644 index 0000000000..61b36cd664 --- /dev/null +++ b/Userland/Applications/SpaceAnalyzer/Tree.cpp @@ -0,0 +1,16 @@ +/* + * Copyright (c) 2022, the SerenityOS developers. + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include "Tree.h" +#include + +void TreeNode::sort_children_by_area() const +{ + if (m_children) { + Vector* children = const_cast*>(m_children.ptr()); + quick_sort(*children, [](auto& a, auto& b) { return b.m_area < a.m_area; }); + } +} diff --git a/Userland/Applications/SpaceAnalyzer/Tree.h b/Userland/Applications/SpaceAnalyzer/Tree.h new file mode 100644 index 0000000000..f5784dfee2 --- /dev/null +++ b/Userland/Applications/SpaceAnalyzer/Tree.h @@ -0,0 +1,45 @@ +/* + * Copyright (c) 2022, the SerenityOS developers. + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include +#include +#include +#include +#include + +struct TreeNode final { + TreeNode(DeprecatedString name) + : m_name(move(name)) {}; + + DeprecatedString name() const { return m_name; } + i64 area() const { return m_area; } + size_t num_children() const + { + if (m_children) { + return m_children->size(); + } + return 0; + } + TreeNode const& child_at(size_t i) const { return m_children->at(i); } + void sort_children_by_area() const; + + DeprecatedString m_name; + i64 m_area { 0 }; + OwnPtr> m_children; +}; + +struct Tree : public RefCounted { + Tree(DeprecatedString root_name) + : m_root(move(root_name)) {}; + ~Tree() {}; + TreeNode m_root; + TreeNode const& root() const + { + return m_root; + }; +}; diff --git a/Userland/Applications/SpaceAnalyzer/TreeMapWidget.cpp b/Userland/Applications/SpaceAnalyzer/TreeMapWidget.cpp index e11aa6347c..e65589f782 100644 --- a/Userland/Applications/SpaceAnalyzer/TreeMapWidget.cpp +++ b/Userland/Applications/SpaceAnalyzer/TreeMapWidget.cpp @@ -36,7 +36,7 @@ static float get_normalized_aspect_ratio(float a, float b) } } -static bool node_is_leaf(TreeMapNode const& node) +static bool node_is_leaf(TreeNode const& node) { return node.num_children() == 0; } @@ -46,7 +46,7 @@ bool TreeMapWidget::rect_can_contain_label(Gfx::IntRect const& rect) const return rect.height() >= font().presentation_size() && rect.width() > 20; } -void TreeMapWidget::paint_cell_frame(GUI::Painter& painter, TreeMapNode const& node, Gfx::IntRect const& cell_rect, Gfx::IntRect const& inner_rect, int depth, HasLabel has_label) const +void TreeMapWidget::paint_cell_frame(GUI::Painter& painter, TreeNode const& node, Gfx::IntRect const& cell_rect, Gfx::IntRect const& inner_rect, int depth, HasLabel has_label) const { if (cell_rect.width() <= 2 || cell_rect.height() <= 2) { painter.fill_rect(cell_rect, Color::Black); @@ -102,7 +102,7 @@ void TreeMapWidget::paint_cell_frame(GUI::Painter& painter, TreeMapNode const& n } template -void TreeMapWidget::lay_out_children(TreeMapNode const& node, Gfx::IntRect const& rect, int depth, Function callback) +void TreeMapWidget::lay_out_children(TreeNode const& node, Gfx::IntRect const& rect, int depth, Function callback) { if (node.num_children() == 0) { return; @@ -215,11 +215,11 @@ void TreeMapWidget::lay_out_children(TreeMapNode const& node, Gfx::IntRect const } } -TreeMapNode const* TreeMapWidget::path_node(size_t n) const +TreeNode const* TreeMapWidget::path_node(size_t n) const { if (!m_tree.ptr()) return nullptr; - TreeMapNode const* iter = &m_tree->root(); + TreeNode const* iter = &m_tree->root(); size_t path_index = 0; while (iter && path_index < m_path.size() && path_index < n) { size_t child_index = m_path[path_index]; @@ -239,13 +239,13 @@ void TreeMapWidget::paint_event(GUI::PaintEvent& event) m_selected_node_cache = path_node(m_path.size()); - TreeMapNode const* node = path_node(m_viewpoint); + TreeNode const* node = path_node(m_viewpoint); if (!node) { painter.fill_rect(frame_inner_rect(), Color::MidGray); } else if (node_is_leaf(*node)) { paint_cell_frame(painter, *node, frame_inner_rect(), Gfx::IntRect(), m_viewpoint - 1, HasLabel::Yes); } else { - lay_out_children(*node, frame_inner_rect(), m_viewpoint, [&](TreeMapNode const& node, int, Gfx::IntRect const& rect, Gfx::IntRect const& inner_rect, int depth, HasLabel has_label, IsRemainder remainder) { + lay_out_children(*node, frame_inner_rect(), m_viewpoint, [&](TreeNode const& node, int, Gfx::IntRect const& rect, Gfx::IntRect const& inner_rect, int depth, HasLabel has_label, IsRemainder remainder) { if (remainder == IsRemainder::No) { paint_cell_frame(painter, node, rect, inner_rect, depth, has_label); } else { @@ -261,12 +261,12 @@ void TreeMapWidget::paint_event(GUI::PaintEvent& event) Vector TreeMapWidget::path_to_position(Gfx::IntPoint position) { - TreeMapNode const* node = path_node(m_viewpoint); + TreeNode const* node = path_node(m_viewpoint); if (!node) { return {}; } Vector path; - lay_out_children(*node, frame_inner_rect(), m_viewpoint, [&](TreeMapNode const&, int index, Gfx::IntRect const& rect, Gfx::IntRect const&, int, HasLabel, IsRemainder is_remainder) { + lay_out_children(*node, frame_inner_rect(), m_viewpoint, [&](TreeNode const&, int index, Gfx::IntRect const& rect, Gfx::IntRect const&, int, HasLabel, IsRemainder is_remainder) { if (is_remainder == IsRemainder::No && rect.contains(position)) { path.append(index); } @@ -283,7 +283,7 @@ void TreeMapWidget::mousemove_event(GUI::MouseEvent& event) } auto* hovered_node = node; - lay_out_children(*node, frame_inner_rect(), m_viewpoint, [&](TreeMapNode const&, int index, Gfx::IntRect const& rect, Gfx::IntRect const&, int, HasLabel, IsRemainder is_remainder) { + lay_out_children(*node, frame_inner_rect(), m_viewpoint, [&](TreeNode const&, int index, Gfx::IntRect const& rect, Gfx::IntRect const&, int, HasLabel, IsRemainder is_remainder) { if (is_remainder == IsRemainder::No && rect.contains(event.position())) { hovered_node = &hovered_node->child_at(index); } @@ -294,7 +294,7 @@ void TreeMapWidget::mousemove_event(GUI::MouseEvent& event) void TreeMapWidget::mousedown_event(GUI::MouseEvent& event) { - TreeMapNode const* node = path_node(m_viewpoint); + TreeNode const* node = path_node(m_viewpoint); if (node && !node_is_leaf(*node)) { Vector path = path_to_position(event.position()); if (!path.is_empty()) { @@ -312,7 +312,7 @@ void TreeMapWidget::doubleclick_event(GUI::MouseEvent& event) { if (event.button() != GUI::MouseButton::Primary) return; - TreeMapNode const* node = path_node(m_viewpoint); + TreeNode const* node = path_node(m_viewpoint); if (node && !node_is_leaf(*node)) { Vector path = path_to_position(event.position()); m_path.shrink(m_viewpoint); @@ -355,7 +355,7 @@ void TreeMapWidget::context_menu_event(GUI::ContextMenuEvent& context_menu_event on_context_menu_request(context_menu_event); } -void TreeMapWidget::set_tree(RefPtr tree) +void TreeMapWidget::set_tree(RefPtr tree) { m_tree = tree; m_path.clear(); diff --git a/Userland/Applications/SpaceAnalyzer/TreeMapWidget.h b/Userland/Applications/SpaceAnalyzer/TreeMapWidget.h index f24406dd6e..82cb6d0fe1 100644 --- a/Userland/Applications/SpaceAnalyzer/TreeMapWidget.h +++ b/Userland/Applications/SpaceAnalyzer/TreeMapWidget.h @@ -6,27 +6,12 @@ #pragma once +#include "Tree.h" #include #include namespace SpaceAnalyzer { -struct TreeMapNode { - virtual DeprecatedString name() const = 0; - virtual i64 area() const = 0; - virtual size_t num_children() const = 0; - virtual TreeMapNode const& child_at(size_t i) const = 0; - virtual void sort_children_by_area() const = 0; - -protected: - virtual ~TreeMapNode() = default; -}; - -struct TreeMap : public RefCounted { - virtual ~TreeMap() = default; - virtual TreeMapNode const& root() const = 0; -}; - class TreeMapWidget final : public GUI::Frame { C_OBJECT(TreeMapWidget) @@ -35,10 +20,10 @@ public: Function on_path_change; Function on_context_menu_request; size_t path_size() const; - TreeMapNode const* path_node(size_t n) const; + TreeNode const* path_node(size_t n) const; size_t viewpoint() const; void set_viewpoint(size_t); - void set_tree(RefPtr tree); + void set_tree(RefPtr tree); private: TreeMapWidget() = default; @@ -62,11 +47,11 @@ private: }; template - void lay_out_children(TreeMapNode const&, Gfx::IntRect const&, int depth, Function); - void paint_cell_frame(GUI::Painter&, TreeMapNode const&, Gfx::IntRect const&, Gfx::IntRect const&, int depth, HasLabel has_label) const; + void lay_out_children(TreeNode const&, Gfx::IntRect const&, int depth, Function); + void paint_cell_frame(GUI::Painter&, TreeNode const&, Gfx::IntRect const&, Gfx::IntRect const&, int depth, HasLabel has_label) const; Vector path_to_position(Gfx::IntPoint); - RefPtr m_tree; + RefPtr m_tree; Vector m_path; size_t m_viewpoint { 0 }; void const* m_selected_node_cache; diff --git a/Userland/Applications/SpaceAnalyzer/main.cpp b/Userland/Applications/SpaceAnalyzer/main.cpp index 558bcf82b9..d234c9bc13 100644 --- a/Userland/Applications/SpaceAnalyzer/main.cpp +++ b/Userland/Applications/SpaceAnalyzer/main.cpp @@ -4,6 +4,7 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include "Tree.h" #include "TreeMapWidget.h" #include #include @@ -38,44 +39,6 @@ static constexpr auto APP_NAME = "Space Analyzer"sv; static constexpr size_t FILES_ENCOUNTERED_UPDATE_STEP_SIZE = 25; -struct TreeNode : public SpaceAnalyzer::TreeMapNode { - TreeNode(DeprecatedString name) - : m_name(move(name)) {}; - - virtual DeprecatedString name() const override { return m_name; } - virtual i64 area() const override { return m_area; } - virtual size_t num_children() const override - { - if (m_children) { - return m_children->size(); - } - return 0; - } - virtual TreeNode const& child_at(size_t i) const override { return m_children->at(i); } - virtual void sort_children_by_area() const override - { - if (m_children) { - Vector* children = const_cast*>(m_children.ptr()); - quick_sort(*children, [](auto& a, auto& b) { return b.m_area < a.m_area; }); - } - } - - DeprecatedString m_name; - i64 m_area { 0 }; - OwnPtr> m_children; -}; - -struct Tree : public SpaceAnalyzer::TreeMap { - Tree(DeprecatedString root_name) - : m_root(move(root_name)) {}; - virtual ~Tree() {}; - TreeNode m_root; - virtual SpaceAnalyzer::TreeMapNode const& root() const override - { - return m_root; - }; -}; - struct MountInfo { DeprecatedString mount_point; DeprecatedString source; @@ -301,7 +264,7 @@ static DeprecatedString get_absolute_path_to_selected_node(SpaceAnalyzer::TreeMa if (k != 0) { path_builder.append('/'); } - SpaceAnalyzer::TreeMapNode const* node = treemapwidget.path_node(k); + TreeNode const* node = treemapwidget.path_node(k); path_builder.append(node->name()); } return path_builder.build(); @@ -416,7 +379,7 @@ ErrorOr serenity_main(Main::Arguments arguments) continue; } - const SpaceAnalyzer::TreeMapNode* node = treemapwidget.path_node(k); + const TreeNode* node = treemapwidget.path_node(k); builder.append('/'); builder.append(node->name());