[gnome-boxes/show-recomended-resources: 3/8] Custom classes for size & string properties

[Zeeshan Ali Khattak <zeeshanak src gnome org>]

commit d05e8d81dbc315aa07a1709574711d8cdf47a084
Author: Zeeshan Ali (Khattak) <zeeshanak gnome org>
Date:   Wed Jan 30 15:48:29 2013 +0100

    Custom classes for size & string properties
    
    Main motivation for this was me being unable to convince myself to add
    yet more arguments to IPropertiesProvider.add_size_property() as too many
    arguments goes against good OO practices[1]. So I think its better to
    have a custom class for size property and replace optional arguments of
    IPropertiesProvider.add_size_property() by fields/props/signals of
    SizeProperty class.
    
    StringProperty then just makes sense to have for consistency.
    
    [1]
    http://stackoverflow.com/questions/4630470/how-many-arguments-is-a-reasonable-number-big-objects-vs-atomic-arguments-oop

 src/i-properties-provider.vala      |  131 +++++++++++++++++++----------------
 src/libvirt-machine-properties.vala |   31 +++++----
 src/remote-machine.vala             |    5 +-
 3 files changed, 91 insertions(+), 76 deletions(-)
---
diff --git a/src/i-properties-provider.vala b/src/i-properties-provider.vala
index 067c7c6..9883a2d 100644
--- a/src/i-properties-provider.vala
+++ b/src/i-properties-provider.vala
@@ -50,88 +50,97 @@ private class Boxes.Property: GLib.Object {
     }
 }
 
-private delegate void PropertyStringChanged (Boxes.Property property, string value) throws Boxes.Error;
-private delegate void PropertySizeChanged (Boxes.Property property, uint64 value) throws Boxes.Error;
+private class Boxes.SizeProperty : Boxes.Property {
+    public signal void changed (uint64 value);
 
-[Flags]
-public enum PropertyCreationFlag {
-    NONE = 0,
-    NO_USB = 1<<0
+    public SizeProperty (string name, uint64 size, uint64 min, uint64 max, uint64 step) {
+        var label = new Gtk.Label (format_size (((uint64) size) * Osinfo.KIBIBYTES, FormatSizeFlags.IEC_UNITS));
+        label.halign = Gtk.Align.CENTER;
+
+        var scale = new Gtk.Scale.with_range (Gtk.Orientation.HORIZONTAL, min, max, step);
+
+        scale.add_mark (min, Gtk.PositionType.BOTTOM, format_size (min * Osinfo.KIBIBYTES, FormatSizeFlags.IEC_UNITS));
+        scale.add_mark (max, Gtk.PositionType.BOTTOM,
+                        "%s (maximum)".printf (format_size (max * Osinfo.KIBIBYTES, FormatSizeFlags.IEC_UNITS)));
+
+        scale.set_show_fill_level (true);
+        scale.set_restrict_to_fill_level (false);
+        scale.set_value (size);
+        scale.set_fill_level (size);
+        scale.set_draw_value (false);
+        scale.hexpand = true;
+        scale.margin_bottom = 20;
+
+        base (name, label, scale);
+
+        scale.value_changed.connect (() => {
+            uint64 v = (uint64) scale.get_value ();
+            label.set_text (format_size (v * Osinfo.KIBIBYTES, FormatSizeFlags.IEC_UNITS));
+            scale.set_fill_level (v);
+
+            changed ((uint64) scale.get_value ());
+        });
+    }
 }
 
-private interface Boxes.IPropertiesProvider: GLib.Object {
-    public abstract List<Boxes.Property> get_properties (Boxes.PropertiesPage page, PropertyCreationFlag flags);
+private class Boxes.StringProperty : Boxes.Property {
+    public signal bool changed (string value);
 
-    protected Boxes.Property add_property (ref List<Boxes.Property> list, string? name, Widget widget, Widget? extra_widget = null) {
-        var property = new Property (name, widget, extra_widget);
-        list.append (property);
-        return property;
+    public bool editable {
+        get { return entry.editable; }
+        set { entry.editable = value; }
     }
 
-    protected Boxes.Property add_string_property (ref List<Boxes.Property>       list,
-                                                  string                         name,
-                                                  string                         value,
-                                                  PropertyStringChanged?         changed = null) {
+    private Boxes.EditableEntry entry;
+
+    public StringProperty (string name, string value) {
         var entry = new Boxes.EditableEntry ();
 
+        base (name, entry, null);
+        this.entry = entry;
+
         entry.text = value;
         entry.selectable = true;
-        entry.editable = changed != null;
 
-        var property = add_property (ref list, name, entry);
         entry.editing_done.connect (() => {
-            try {
-                changed (property, entry.text);
-            } catch (Boxes.Error.INVALID error) {
+            if (!changed (entry.text))
                 entry.start_editing ();
-            } catch (Boxes.Error error) {
-                warning (error.message);
-            }
         });
-
-        return property;
     }
+}
 
-    protected Boxes.Property add_size_property (ref List<Boxes.Property>       list,
-                                                string                         name,
-                                                uint64                         size,
-                                                uint64                         min,
-                                                uint64                         max,
-                                                uint64                         step,
-                                                PropertySizeChanged?           changed = null) {
-        var label = new Gtk.Label (format_size (((uint64) size) * Osinfo.KIBIBYTES, FormatSizeFlags.IEC_UNITS));
-        label.halign = Gtk.Align.CENTER;
+[Flags]
+public enum PropertyCreationFlag {
+    NONE = 0,
+    NO_USB = 1<<0
+}
 
-        var scale = new Gtk.Scale.with_range (Gtk.Orientation.HORIZONTAL, min, max, step);
+private interface Boxes.IPropertiesProvider: GLib.Object {
+    public abstract List<Boxes.Property> get_properties (Boxes.PropertiesPage page, PropertyCreationFlag flags);
 
-        scale.add_mark (min, Gtk.PositionType.BOTTOM,
-                        format_size (min * Osinfo.KIBIBYTES, FormatSizeFlags.IEC_UNITS));
-        scale.add_mark (max, Gtk.PositionType.BOTTOM,
-                        "%s (maximum)".printf (format_size (max * Osinfo.KIBIBYTES, FormatSizeFlags.IEC_UNITS)));
+    protected Boxes.Property add_property (ref List<Boxes.Property> list, string? name, Widget widget, Widget? extra_widget = null) {
+        var property = new Property (name, widget, extra_widget);
+        list.append (property);
+        return property;
+    }
 
-        scale.value_changed.connect (() => {
-                uint64 v = (uint64)scale.get_value ();
-                label.set_text (format_size (v * Osinfo.KIBIBYTES, FormatSizeFlags.IEC_UNITS));
-                scale.set_fill_level (v);
-        });
+    protected Boxes.StringProperty add_string_property (ref List<Boxes.Property> list,
+                                                        string                   name,
+                                                        string                   value) {
+        var property = new StringProperty (name, value);
+        list.append (property);
 
-        scale.set_show_fill_level (true);
-        scale.set_restrict_to_fill_level (false);
-        scale.set_value (size);
-        scale.set_fill_level (size);
-        scale.set_draw_value (false);
-        scale.hexpand = true;
-        scale.margin_bottom = 20;
+        return property;
+    }
 
-        var property = add_property (ref list, name, label, scale);
-        if (changed != null)
-            scale.value_changed.connect (() => {
-                try {
-                    changed (property, (uint64) scale.get_value ());
-                } catch (Boxes.Error error) {
-                    warning (error.message);
-                }
-            });
+    protected Boxes.SizeProperty add_size_property (ref List<Boxes.Property> list,
+                                                    string                   name,
+                                                    uint64                   size,
+                                                    uint64                   min,
+                                                    uint64                   max,
+                                                    uint64                   step) {
+        var property = new SizeProperty (name, size, min, max, step);
+        list.append (property);
 
         return property;
     }
diff --git a/src/libvirt-machine-properties.vala b/src/libvirt-machine-properties.vala
index 871db7c..7e2a0c1 100644
--- a/src/libvirt-machine-properties.vala
+++ b/src/libvirt-machine-properties.vala
@@ -15,7 +15,7 @@ private abstract class Boxes.LibvirtMachineProperties: Boxes.Machine {
         base (source, name);
     }
 
-    public void try_change_name (string name) throws Boxes.Error {
+    public bool try_change_name (string name) {
         try {
             var config = domain.get_config (GVir.DomainXMLFlags.INACTIVE);
             // Te use libvirt "title" for free form user name
@@ -24,10 +24,11 @@ private abstract class Boxes.LibvirtMachineProperties: Boxes.Machine {
             domain.set_config (config);
 
             this.name = name;
+            return true;
         } catch (GLib.Error error) {
             warning ("Failed to change title of box '%s' to '%s': %s",
                      domain.get_name (), name, error.message);
-            throw new Boxes.Error.INVALID ("Invalid libvirt title");
+            return false;
         }
     }
 
@@ -115,12 +116,14 @@ private abstract class Boxes.LibvirtMachineProperties: Boxes.Machine {
 
         switch (page) {
         case PropertiesPage.LOGIN:
-            add_string_property (ref list, _("Name"), name, (property, name) => {
-                try_change_name (name);
+            var property = add_string_property (ref list, _("Name"), name);
+            property.editable = true;
+            property.changed.connect ((property, name) => {
+                return try_change_name (name);
             });
             add_string_property (ref list, _("Virtualizer"), source.uri);
             if (display != null)
-                add_string_property (ref list, _("URI"), display.uri);
+                property = add_string_property (ref list, _("URI"), display.uri);
             break;
 
         case PropertiesPage.SYSTEM:
@@ -390,8 +393,8 @@ private abstract class Boxes.LibvirtMachineProperties: Boxes.Machine {
                                               domain_config.memory,
                                               Osinfo.MEBIBYTES / Osinfo.KIBIBYTES,
                                               max_ram,
-                                              Osinfo.MEBIBYTES / Osinfo.KIBIBYTES,
-                                              on_ram_changed);
+                                              Osinfo.MEBIBYTES / Osinfo.KIBIBYTES);
+            property.changed.connect (on_ram_changed);
 
             this.notify["state"].connect (() => {
                 if (!is_on ())
@@ -491,13 +494,13 @@ private abstract class Boxes.LibvirtMachineProperties: Boxes.Machine {
             var pool_info = pool.get_info ();
             var max_storage = (volume_info.capacity + pool_info.available)  / Osinfo.KIBIBYTES;
 
-            add_size_property (ref list,
-                               _("Maximum Disk Size"),
-                               volume_info.capacity / Osinfo.KIBIBYTES,
-                               volume_info.capacity / Osinfo.KIBIBYTES,
-                               max_storage,
-                               Osinfo.GIBIBYTES / Osinfo.KIBIBYTES,
-                               on_storage_changed);
+            var property = add_size_property (ref list,
+                                              _("Maximum Disk Size"),
+                                              volume_info.capacity / Osinfo.KIBIBYTES,
+                                              volume_info.capacity / Osinfo.KIBIBYTES,
+                                              max_storage,
+                                              Osinfo.GIBIBYTES / Osinfo.KIBIBYTES);
+            property.changed.connect (on_storage_changed);
         } catch (GLib.Error error) {
             warning ("Failed to get information on volume '%s' or it's parent pool: %s",
                      storage_volume.get_name (),
diff --git a/src/remote-machine.vala b/src/remote-machine.vala
index 2e3c412..822d257 100644
--- a/src/remote-machine.vala
+++ b/src/remote-machine.vala
@@ -49,8 +49,11 @@ private class Boxes.RemoteMachine: Boxes.Machine, Boxes.IPropertiesProvider {
 
         switch (page) {
         case PropertiesPage.LOGIN:
-            add_string_property (ref list, _("Name"), source.name, (property, name) => {
+            var property = add_string_property (ref list, _("Name"), source.name);
+            property.editable = true;
+            property.changed.connect ((property, name) => {
                 source.name = name;
+                return true;
             });
             add_string_property (ref list, _("URI"), source.uri);
             break;