GNOME.org
 

[glom] Don't take a shared_ptr<> when the method does not share ownership.

commit 92f8c3f79a118adc7fda2c8d408ac48f1ef065ba
Author: Murray Cumming <murrayc murrayc com>
Date:   Tue Feb 9 14:55:40 2016 +0100

    Don't take a shared_ptr<> when the method does not share ownership.
    
    As suggested by recent talks by:
    
    Bjarne Stroustrup: 2015: Writing Good C++14:
    video: https://www.youtube.com/watch?v=1OEu9C51K2A
    slide: 
https://github.com/isocpp/CppCoreGuidelines/blob/master/talks/Stroustrup%20-%20CppCon%202015%20keynote.pdf
    
    C++ Core Guidelines:
    "F.7: For general use, take T* or T& arguments rather than smart pointers":
    
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#f7-for-general-use-take-t-or-t-arguments-rather-than-smart-pointers

 glom/libglom/document/document.cc                |    4 ++--
 glom/libglom/document/document.h                 |    2 +-
 glom/mode_data/datawidget/datawidget.cc          |    2 +-
 glom/mode_data/flowtablewithfields.cc            |    4 ++--
 glom/mode_design/layout/combobox_relationship.cc |    5 ++++-
 glom/print_layout/canvas_layout_item.cc          |   13 +++++--------
 glom/print_layout/canvas_layout_item.h           |    4 ++--
 glom/print_layout/canvas_print_layout.cc         |    8 ++++----
 glom/utility_widgets/layoutwidgetbase.cc         |    9 +++------
 glom/utility_widgets/layoutwidgetbase.h          |    2 +-
 10 files changed, 25 insertions(+), 28 deletions(-)
---
diff --git a/glom/libglom/document/document.cc b/glom/libglom/document/document.cc
index e325185..83e766b 100644
--- a/glom/libglom/document/document.cc
+++ b/glom/libglom/document/document.cc
@@ -586,9 +586,9 @@ std::shared_ptr<TableInfo> Document::create_table_system_preferences(type_vec_fi
   return prefs_table_info;
 }
 
-bool Document::get_relationship_is_system_properties(const std::shared_ptr<const Relationship>& relationship)
+bool Document::get_relationship_is_system_properties(const Relationship& relationship)
 {
-  return relationship && (relationship->get_name() == GLOM_RELATIONSHIP_NAME_SYSTEM_PROPERTIES);
+  return (relationship.get_name() == GLOM_RELATIONSHIP_NAME_SYSTEM_PROPERTIES);
 }
 
 std::shared_ptr<Relationship> Document::get_relationship(const Glib::ustring& table_name, const 
Glib::ustring& relationship_name) const
diff --git a/glom/libglom/document/document.h b/glom/libglom/document/document.h
index 2ad2bcd..6d20d5c 100644
--- a/glom/libglom/document/document.h
+++ b/glom/libglom/document/document.h
@@ -491,7 +491,7 @@ public:
   static std::shared_ptr<TableInfo> create_table_system_preferences();
   static std::shared_ptr<TableInfo> create_table_system_preferences(type_vec_fields& fields);
   static std::shared_ptr<Relationship> create_relationship_system_preferences(const Glib::ustring& 
table_name);
-  static bool get_relationship_is_system_properties(const std::shared_ptr<const Relationship>& relationship);
+  static bool get_relationship_is_system_properties(const Relationship& relationship);
 #endif //SWIG
 
   /// Failure codes that could be returned by load_after()
diff --git a/glom/mode_data/datawidget/datawidget.cc b/glom/mode_data/datawidget/datawidget.cc
index 443feb2..52b7ae6 100644
--- a/glom/mode_data/datawidget/datawidget.cc
+++ b/glom/mode_data/datawidget/datawidget.cc
@@ -179,7 +179,7 @@ DataWidget::DataWidget(const std::shared_ptr<LayoutItem_Field>& field, const Gli
   if(m_child)
   {
     //Use the text formatting:
-    apply_formatting(*m_child, field);
+    apply_formatting(*m_child, *field);
 
     bool child_added = false; //Don't use an extra container unless necessary.
 
diff --git a/glom/mode_data/flowtablewithfields.cc b/glom/mode_data/flowtablewithfields.cc
index 6c4201f..cea8416 100644
--- a/glom/mode_data/flowtablewithfields.cc
+++ b/glom/mode_data/flowtablewithfields.cc
@@ -564,7 +564,7 @@ void FlowTableWithFields::add_button(const std::shared_ptr<LayoutItem_Button>& l
 
   add_widgets(*widget_to_add, expand);
 
-  apply_formatting(*button, layoutitem_button);
+  apply_formatting(*button, *layoutitem_button);
 }
 
 void FlowTableWithFields::add_textobject(const std::shared_ptr<LayoutItem_Text>& layoutitem_text, const 
Glib::ustring& table_name)
@@ -581,7 +581,7 @@ void FlowTableWithFields::add_textobject(const std::shared_ptr<LayoutItem_Text>&
   label->set_valign(Gtk::ALIGN_CENTER);
   label->show();
 
-  apply_formatting(*label, layoutitem_text);
+  apply_formatting(*label, *layoutitem_text);
 
   add_layoutwidgetbase(label);
 
diff --git a/glom/mode_design/layout/combobox_relationship.cc 
b/glom/mode_design/layout/combobox_relationship.cc
index f88c027..c5cca5e 100644
--- a/glom/mode_design/layout/combobox_relationship.cc
+++ b/glom/mode_design/layout/combobox_relationship.cc
@@ -158,6 +158,9 @@ void ComboBox_Relationship::set_relationships(const std::shared_ptr<Document>& d
   //Fill the model:
   for(const auto& rel : document->get_relationships(parent_table_name, true /* plus system properties */))
   {
+    if(!rel)
+      continue;
+
     auto tree_iter = m_model->append();
     Gtk::TreeModel::Row row = *tree_iter;
 
@@ -165,7 +168,7 @@ void ComboBox_Relationship::set_relationships(const std::shared_ptr<Document>& d
     row[m_model_columns.m_separator] = false;
 
     //Children:
-    if(show_related_relationships && !Document::get_relationship_is_system_properties(rel))
+    if(show_related_relationships && !Document::get_relationship_is_system_properties(*rel))
     {
       for(const auto& sub_rel : document->get_relationships(rel->get_to_table(), false /* plus system 
properties */))
       {
diff --git a/glom/print_layout/canvas_layout_item.cc b/glom/print_layout/canvas_layout_item.cc
index 1620e91..00fe0f3 100644
--- a/glom/print_layout/canvas_layout_item.cc
+++ b/glom/print_layout/canvas_layout_item.cc
@@ -58,21 +58,18 @@ std::shared_ptr<LayoutItem> CanvasLayoutItem::get_layout_item()
   return m_layout_item;
 }
 
-void CanvasLayoutItem::apply_formatting(const Glib::RefPtr<CanvasTextMovable>& canvas_item, const 
std::shared_ptr<const LayoutItem_WithFormatting>& layout_item)
+void CanvasLayoutItem::apply_formatting(const Glib::RefPtr<CanvasTextMovable>& canvas_item, const 
LayoutItem_WithFormatting& layout_item)
 {
   if(!canvas_item)
     return;
 
-  if(!layout_item)
-    return;
-
   //Horizontal alignment:
   const Formatting::HorizontalAlignment alignment =
-    layout_item->get_formatting_used_horizontal_alignment();
+    layout_item.get_formatting_used_horizontal_alignment();
   const Pango::Alignment x_align = (alignment == Formatting::HorizontalAlignment::LEFT ? Pango::ALIGN_LEFT : 
Pango::ALIGN_RIGHT);
   canvas_item->property_alignment() = x_align;
 
-  const auto formatting = layout_item->get_formatting_used();
+  const auto formatting = layout_item.get_formatting_used();
 
   Glib::ustring font = formatting.get_text_format_font();
   if(font.empty())
@@ -162,7 +159,7 @@ Glib::RefPtr<CanvasItemMovable> CanvasLayoutItem::create_canvas_item_for_layout_
     auto canvas_item = CanvasTextMovable::create();
     canvas_item->property_line_width() = 0;
 
-    apply_formatting(canvas_item, text);
+    apply_formatting(canvas_item, *text);
 
     canvas_item->set_text(text->get_text(AppWindow::get_current_locale()));
     child = canvas_item;
@@ -226,7 +223,7 @@ Glib::RefPtr<CanvasItemMovable> CanvasLayoutItem::create_canvas_item_for_layout_
           {
             auto canvas_item = CanvasTextMovable::create();
             canvas_item->property_line_width() = 0;
-            apply_formatting(canvas_item, field);
+            apply_formatting(canvas_item, *field);
 
             Glib::ustring name = field->get_name();
             if(name.empty())
diff --git a/glom/print_layout/canvas_layout_item.h b/glom/print_layout/canvas_layout_item.h
index 93efa03..a3c8d32 100644
--- a/glom/print_layout/canvas_layout_item.h
+++ b/glom/print_layout/canvas_layout_item.h
@@ -78,8 +78,8 @@ private:
   /// Create the appropriate inner canvas item to represent the layout item.
   static Glib::RefPtr<CanvasItemMovable> create_canvas_item_for_layout_item(const 
std::shared_ptr<LayoutItem>& layout_item);
 
-  static void apply_formatting(const Glib::RefPtr<CanvasTextMovable>& canvas_item, const 
std::shared_ptr<const LayoutItem_WithFormatting>& layout_item);
-
+  static void apply_formatting(const Glib::RefPtr<CanvasTextMovable>& canvas_item, const 
LayoutItem_WithFormatting& layout_item);
+  
   static void add_portal_rows_if_necessary(const Glib::RefPtr<CanvasTableMovable>& canvas_table, const 
std::shared_ptr<LayoutItem_Portal>& portal, guint rows_count);
 
   void on_resized();
diff --git a/glom/print_layout/canvas_print_layout.cc b/glom/print_layout/canvas_print_layout.cc
index 55d16b5..1ec0898 100644
--- a/glom/print_layout/canvas_print_layout.cc
+++ b/glom/print_layout/canvas_print_layout.cc
@@ -708,16 +708,16 @@ void Canvas_PrintLayout::fill_with_data_system_preferences(const Glib::RefPtr<Ca
   bool empty = true;
   if(!layoutitem_field->get_name().empty())
   {
-    const auto relationship =
-      layoutitem_field->get_relationship();
-
     if(!document)
     {
       std::cerr << G_STRFUNC << ": document is null\n";
       return;
     }
 
-    if(document->get_relationship_is_system_properties(relationship))
+    const auto relationship =
+      layoutitem_field->get_relationship();
+
+    if(relationship && document->get_relationship_is_system_properties(*relationship))
       empty = false;
   }
 
diff --git a/glom/utility_widgets/layoutwidgetbase.cc b/glom/utility_widgets/layoutwidgetbase.cc
index ead3079..2a88f42 100644
--- a/glom/utility_widgets/layoutwidgetbase.cc
+++ b/glom/utility_widgets/layoutwidgetbase.cc
@@ -80,7 +80,7 @@ void LayoutWidgetBase::set_read_only(bool /* read_only */)
 {
 }
 
-void LayoutWidgetBase::apply_formatting(Gtk::Widget& widget, const std::shared_ptr<const 
LayoutItem_WithFormatting>& layout_item)
+void LayoutWidgetBase::apply_formatting(Gtk::Widget& widget, const LayoutItem_WithFormatting& layout_item)
 {
   auto widget_to_change = &widget;
 
@@ -103,15 +103,12 @@ void LayoutWidgetBase::apply_formatting(Gtk::Widget& widget, const std::shared_p
     return;
   }
 
-  if(!layout_item)
-    return;
-
   //Set justification on labels and text views:
   //Assume that people want left/right justification of multi-line text if they chose
   //left/right alignment of the text itself.
   {
     const Formatting::HorizontalAlignment alignment =
-     layout_item->get_formatting_used_horizontal_alignment(true /* for details view */);
+     layout_item.get_formatting_used_horizontal_alignment(true /* for details view */);
     const Gtk::Justification justification =
       (alignment == Formatting::HorizontalAlignment::LEFT ? Gtk::JUSTIFY_LEFT : Gtk::JUSTIFY_RIGHT);
     const Gtk::Align x_align =
@@ -145,7 +142,7 @@ void LayoutWidgetBase::apply_formatting(Gtk::Widget& widget, const std::shared_p
     }
   }
 
-  const auto formatting = layout_item->get_formatting_used();
+  const auto formatting = layout_item.get_formatting_used();
 
   //Use the text formatting:
   const auto font_desc = formatting.get_text_format_font();
diff --git a/glom/utility_widgets/layoutwidgetbase.h b/glom/utility_widgets/layoutwidgetbase.h
index 93a5f7d..b22d54c 100644
--- a/glom/utility_widgets/layoutwidgetbase.h
+++ b/glom/utility_widgets/layoutwidgetbase.h
@@ -81,7 +81,7 @@ protected:
   virtual AppWindow* get_appwindow() const; // = 0;
 
 
-  static void apply_formatting(Gtk::Widget& widget, const std::shared_ptr<const LayoutItem_WithFormatting>& 
layout_item);
+  static void apply_formatting(Gtk::Widget& widget, const LayoutItem_WithFormatting& layout_item);
 
 protected: //TODO: Add accessor?
   std::shared_ptr<LayoutItem> m_layout_item;
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]