Merge lp:~thumper/nux/properties into lp:nux

[Tim Penhey]

This is nux::Properties Mark II.

The fundamental change here is that nux::Property is no longer constrained by
type, and it is not serializable. If you want a serializable property, use
the nux::SerializableProperty. This inherits from nux::Property.
SerializableProperty instances still require the type traits to be set, and as
such are limited in content type. nux::Property can now hold class instances
or smart pointers - in fact anything that implements a != operator.

This branch also introduces several nifty new changes:
 * nux::Property can now have a custom setter method
 * nux::ROProperty is a read only property provided by a method
 * nux::RWProperty is a read/write property with implementation provided by
 methods provided as slots.

I also changed the method names to be capitalised to conform to the coding
standard.

I also added a nice attribution to Lois, who was the author of the paper I
used as a basis for the design.

I've added many tests for this, and to get a good feel for how to use the
properties, you should look at the tests.

Tests of intereste are the following (IMO):
 * TestProperty.TestSetterConstructor
 * TestProperty.TestCustomSetterFunction
 * TestROProperty.TestGetterConstructor
 * TestROProperty.TestSetGetter
 * TestRWProperty.TestFunctionConstructor
 * TestRWProperty.TestPimplClassExample

[Mikkel Kamstrup Erlandsen (kamstrup)]

Had a quick skim - nice work Tim :-) I have two points I want to raise:

 a) If I understand correct the default behaviour for a ROProperty is to not signal on change. I think this is this is the wrong behaviour for two reasons:
   1) From my experience RO properties are used for things like a bool "connected" or "mapped" where they reflect unchangeable aspects of the underlying system. Normally you'll want change notifications on those.
   2) It is dangerous refactoring-wise that changing a property into a RWProperty causes a change in semantics. I can see many hours of lost debugging there - "why doesn't XYZ update anymore?!" :-)

Consumers that want to do changes without notifications can just change the underlying private member directly, and if you're worried about overhead for setting up sigc++ signals then one can maybe get away with some form of lazy setup?

 b) This one is not necessary for a first cut but we might wanna think about it at this stage nonetheless: In glib you have g_object_freeze/thaw_notify() which twiddles a freeze count +-1. If the freeze count is >0 all change notifications are queued up only to be emitted once it reaches 0. This is super useful to avoid reentrancy and threading issues. On GObjects the freeze count is global across all properties per instance, but one could imagine a per-property approach as well.
  - Anyway, *I* like (and use) the freeze/thaw functionality a lot, but I bet someone considers it a dangerous exposed implementation detail of GObject :-)

[Tim Penhey (thumper)]

On Mon, 11 Jul 2011 19:31:21 you wrote:
> Had a quick skim - nice work Tim :-) I have two points I want to raise:
>
> a) If I understand correct the default behaviour for a ROProperty is to
> not signal on change. I think this is this is the wrong behaviour for two
> reasons: 1) From my experience RO properties are used for things like a
> bool "connected" or "mapped" where they reflect unchangeable aspects of
> the underlying system. Normally you'll want change notifications on those.

My only real issue with ROProperty is that we don't know if it has changed or
not. So how do we know to signal?

[Mikkel Kamstrup Erlandsen (kamstrup)]

Good point. Conceptually I'd say that one needs a private or protected
setter function that does this (which must be used by the consumers
just like a public setter would), but if I understand correctly
ROProps don't have setters in this setup?

[Tim Penhey (thumper)]

On Mon, 11 Jul 2011 21:46:29 you wrote:
> Good point. Conceptually I'd say that one needs a private or protected
> setter function that does this (which must be used by the consumers
> just like a public setter would), but if I understand correctly
> ROProps don't have setters in this setup?

Exactly. Given that the property is a member itself, you can't have a
protected setter that is accessible by the containing class.

[Mikkel Kamstrup Erlandsen (kamstrup)]

Also - a clarification: If I read correctly the Property objects live per instance and not per class. In other words two instances a1 and a2 of a class A will both have an instance of the Property objects for A? The conventional way to do properties afaik is to have the properties live in the class scope instead avoiding the additional churn of the per-instance setup/teardown. Can you elaborate?

[Neil J. Patel (njpatel)]

The owner would emit the signal when they change the underlying store. As long as its possible to emit the changed signal, that should cover the use case that Mikkel mentioned.

Sent from my iPhone

On 11 Jul 2011, at 10:36, Tim Penhey <email address hidden> wrote:

> On Mon, 11 Jul 2011 19:31:21 you wrote:
>> Had a quick skim - nice work Tim :-) I have two points I want to raise:
>>
>> a) If I understand correct the default behaviour for a ROProperty is to
>> not signal on change. I think this is this is the wrong behaviour for two
>> reasons: 1) From my experience RO properties are used for things like a
>> bool "connected" or "mapped" where they reflect unchangeable aspects of
>> the underlying system. Normally you'll want change notifications on those.
>
> My only real issue with ROProperty is that we don't know if it has changed or
> not. So how do we know to signal?
>
> --
> https://code.launchpad.net/~thumper/nux/properties/+merge/67479
> Your team Unity Team is requested to review the proposed merge of lp:~thumper/nux/properties into lp:nux.

[Mikkel Kamstrup Erlandsen (kamstrup)]

Neil, sure, that's my whole point :-) My point is that the following
change gives a lot of code to refactor:

 TeaCup::TeaCup() {
- _full_prop = new RWProperty<bool> ();
+ _full_prop = new ROProperty<bool> ();
 }

You need to refactor all Set() calls and make sure you manually emit
signals where appropriate. I was just arguing for a solution where the
above diff would be all there was to it.

Anyway. Maybe I am just bikeshedding this :-)

[Tim Penhey (thumper)]

On Tue, 12 Jul 2011 02:43:23 you wrote:
> The owner would emit the signal when they change the underlying store. As
> long as its possible to emit the changed signal, that should cover the use
> case that Mikkel mentioned.

What about the use case where the property is entirely calculated?

The result may well change but no changed event emitted.

This is why I didn't have the ROProperty inherit from the event base. There
is no changed event to connect to, so no expectation of changes.

I added the ROProperty class because Neil had said he'd like one, but I have a
feeling that the basic Property class with a custom set method covers 99% of
our desired use cases for simple properties.

Tim

[Tim Penhey (thumper)]

On Tue, 12 Jul 2011 07:41:23 you wrote:
> Neil, sure, that's my whole point :-) My point is that the following
> change gives a lot of code to refactor:
>
> TeaCup::TeaCup() {
> - _full_prop = new RWProperty<bool> ();
> + _full_prop = new ROProperty<bool> ();
> }

Aside: you don't use new with properties.

>
> You need to refactor all Set() calls and make sure you manually emit
> signals where appropriate. I was just arguing for a solution where the
> above diff would be all there was to it.

There are no Set calls for a ROProperty. If you are changing a RWProperty (or
Property) into a ROProperty, then yes you should refactor as the underlying
semantics have changed, and the usage sites should be looked at and evaluated.

> Anyway. Maybe I am just bikeshedding this :-)

Perhaps :-)

[Tim Penhey (thumper)]

On Mon, 11 Jul 2011 22:32:33 you wrote:
> Also - a clarification: If I read correctly the Property objects live per
> instance and not per class. In other words two instances a1 and a2 of a
> class A will both have an instance of the Property objects for A?

Yes.

> The conventional way to do properties afaik is to have the properties
> live in the class scope instead avoiding the additional churn of the
> per-instance setup/teardown. Can you elaborate?

Properties are supposed to be like special member variables, most often
public, that have changed events associated with them.

You can have static Property objects that belong to a class as well if you'd
like.

[Mikkel Kamstrup Erlandsen (kamstrup)]

Ok, I know I am beating a dead horse here; you two agree so I am good
with that :-)

Nonetheless, I don't think my point has come across: My worry is that
this is a purely dynamical property system and that it is exactly
*unlike* member variables in that they are not related to (let alone
defined in) the class. So two instances a1 and a2 of class A could
easily have totally different sets of properties. I wanted to avoid
this by somehow making the property specs a part of the class
definition in the .h file (probably by using some magical static
initializers). This would also make property setup slightly
(massively?) lighter at runtime because it would be per class and not
per instance.

[Tim Penhey (thumper)]

On Tue, 12 Jul 2011 20:07:40 you wrote:
> Ok, I know I am beating a dead horse here; you two agree so I am good
> with that :-)
>
> Nonetheless, I don't think my point has come across: My worry is that
> this is a purely dynamical property system and that it is exactly
> *unlike* member variables in that they are not related to (let alone
> defined in) the class.

Ah... I think this is where we disagree. It isn't a dynamical property
system.

> So two instances a1 and a2 of class A could
> easily have totally different sets of properties.

No they couldn't.

> I wanted to avoid
> this by somehow making the property specs a part of the class
> definition in the .h file (probably by using some magical static
> initializers). This would also make property setup slightly
> (massively?) lighter at runtime because it would be per class and not
> per instance.

[Mikkel Kamstrup Erlandsen (kamstrup)]

On 12 July 2011 11:16, Tim Penhey <email address hidden> wrote:
> On Tue, 12 Jul 2011 20:07:40 you wrote:
>> Ok, I know I am beating a dead horse here; you two agree so I am good
>> with that :-)
>>
>> Nonetheless, I don't think my point has come across: My worry is that
>> this is a purely dynamical property system and that it is exactly
>> *unlike* member variables in that they are not related to (let alone
>> defined in) the class.
>
> Ah... I think this is where we disagree.  It isn't a dynamical property
> system.
>
>> So two instances a1 and a2 of class A could
>> easily have totally different sets of properties.
>
> No they couldn't.

Hehe... and my mediocre C++ skills are revealed - Move along, nothing
to see here :-)

[Jay Taoko (jaytaoko)]

I spend most of the review on the tests as they are the easier to understand.

review: Approve

[lp:~thumper/nux/properties]

384. By Tim Penhey on 2011-07-13

Land nux::Properties Mark II.

=== modified file 'NuxCore/Property-inl.h'
--- NuxCore/Property-inl.h	2011-06-27 14:18:42 +0000
+++ NuxCore/Property-inl.h	2011-07-11 02:16:37 +0000
@@ -24,165 +24,321 @@
 
 namespace nux {
 
-template <typename VALUE_TYPE>
-ConnectableProperty<VALUE_TYPE>::ConnectableProperty()
-  : value_(VALUE_TYPE())
-  , notify_(true)
-{}
-
-template <typename VALUE_TYPE>
-ConnectableProperty<VALUE_TYPE>::ConnectableProperty(VALUE_TYPE const& initial)
-  : value_(initial)
-  , notify_(true)
-{}
-
-template <typename VALUE_TYPE>
-VALUE_TYPE const& ConnectableProperty<VALUE_TYPE>::operator=(VALUE_TYPE const& value)
-{
-  set(value);
-  return value_;
-}
-
-template <typename VALUE_TYPE>
-ConnectableProperty<VALUE_TYPE>::operator VALUE_TYPE const&() const
-{
-  return value_;
-}
-
-template <typename VALUE_TYPE>
-VALUE_TYPE const& ConnectableProperty<VALUE_TYPE>::operator()() const
-{
-  return value_;
-}
-
-template <typename VALUE_TYPE>
-void ConnectableProperty<VALUE_TYPE>::operator()(VALUE_TYPE const& value)
-{
-  set(value);
-}
-
-template <typename VALUE_TYPE>
-void ConnectableProperty<VALUE_TYPE>::disable_notifications()
+
+template <typename VALUE_TYPE>
+PropertyChangedSignal<VALUE_TYPE>::PropertyChangedSignal()
+  : notify_(true)
+{}
+
+template <typename VALUE_TYPE>
+void PropertyChangedSignal<VALUE_TYPE>::DisableNotifications()
 {
   notify_ = false;
 }
 
 template <typename VALUE_TYPE>
-void ConnectableProperty<VALUE_TYPE>::enable_notifications()
+void PropertyChangedSignal<VALUE_TYPE>::EnableNotifications()
 {
   notify_ = true;
 }
 
-  // get and set access
-template <typename VALUE_TYPE>
-VALUE_TYPE const& ConnectableProperty<VALUE_TYPE>::get() const
-{
-  return value_;
-}
-
-template <typename VALUE_TYPE>
-void ConnectableProperty<VALUE_TYPE>::set(VALUE_TYPE const& value)
-{
-  if (value != value_) {
-    value_ = value;
-    if (notify_) {
-      changed.emit(value_);
-    }
-  }
-}
+template <typename VALUE_TYPE>
+void PropertyChangedSignal<VALUE_TYPE>::EmitChanged(VALUE_TYPE const& new_value)
+{
+  if (notify_)
+    changed.emit(new_value);
+}
+
+
+template <typename VALUE_TYPE>
+Property<VALUE_TYPE>::Property()
+  : value_(VALUE_TYPE())
+  , setter_function_(sigc::mem_fun(this, &Property<VALUE_TYPE>::DefaultSetter))
+{}
+
+template <typename VALUE_TYPE>
+Property<VALUE_TYPE>::Property(VALUE_TYPE const& initial)
+  : value_(initial)
+  , setter_function_(sigc::mem_fun(this, &Property<VALUE_TYPE>::DefaultSetter))
+{}
+
+template <typename VALUE_TYPE>
+Property<VALUE_TYPE>::Property(VALUE_TYPE const& initial,
+                               SetterFunction setter_function)
+  : value_(initial)
+  , setter_function_(setter_function)
+{}
+
+template <typename VALUE_TYPE>
+VALUE_TYPE Property<VALUE_TYPE>::operator=(VALUE_TYPE const& value)
+{
+  return Set(value);
+}
+
+template <typename VALUE_TYPE>
+Property<VALUE_TYPE>::operator VALUE_TYPE() const
+{
+  return value_;
+}
+
+template <typename VALUE_TYPE>
+VALUE_TYPE Property<VALUE_TYPE>::operator()() const
+{
+  return value_;
+}
+
+template <typename VALUE_TYPE>
+VALUE_TYPE Property<VALUE_TYPE>::operator()(VALUE_TYPE const& value)
+{
+  return Set(value);
+}
+
+template <typename VALUE_TYPE>
+VALUE_TYPE Property<VALUE_TYPE>::Get() const
+{
+  return value_;
+}
+
+template <typename VALUE_TYPE>
+VALUE_TYPE Property<VALUE_TYPE>::Set(VALUE_TYPE const& value)
+{
+  if (setter_function_(value_, value))
+    SignalBase::EmitChanged(value_);
+  return value_;
+}
+
+template <typename VALUE_TYPE>
+bool Property<VALUE_TYPE>::DefaultSetter(VALUE_TYPE& target,
+                                         VALUE_TYPE const& value)
+{
+  bool changed = false;
+  if (target != value) {
+    target = value;
+    changed = true;
+  }
+  return changed;
+}
+
+template <typename VALUE_TYPE>
+void Property<VALUE_TYPE>::SetSetterFunction(SetterFunction setter_function)
+{
+  setter_function_ = setter_function;
+}
+
+
+template <typename VALUE_TYPE>
+ROProperty<VALUE_TYPE>::ROProperty()
+  : getter_function_(sigc::mem_fun(this, &ROProperty<VALUE_TYPE>::DefaultGetter))
+{}
+
+template <typename VALUE_TYPE>
+ROProperty<VALUE_TYPE>::ROProperty(GetterFunction getter_function)
+  : getter_function_(getter_function)
+{}
+
+template <typename VALUE_TYPE>
+ROProperty<VALUE_TYPE>::operator VALUE_TYPE() const
+{
+  return getter_function_();
+}
+
+template <typename VALUE_TYPE>
+VALUE_TYPE ROProperty<VALUE_TYPE>::operator()() const
+{
+  return getter_function_();
+}
+
+template <typename VALUE_TYPE>
+VALUE_TYPE ROProperty<VALUE_TYPE>::Get() const
+{
+  return getter_function_();
+}
+
+template <typename VALUE_TYPE>
+VALUE_TYPE ROProperty<VALUE_TYPE>::DefaultGetter() const
+{
+  return VALUE_TYPE();
+}
+
+template <typename VALUE_TYPE>
+void ROProperty<VALUE_TYPE>::SetGetterFunction(GetterFunction getter_function)
+{
+  getter_function_ = getter_function;
+}
+
+
+template <typename VALUE_TYPE>
+RWProperty<VALUE_TYPE>::RWProperty()
+  : getter_function_(sigc::mem_fun(this, &RWProperty<VALUE_TYPE>::DefaultGetter))
+  , setter_function_(sigc::mem_fun(this, &RWProperty<VALUE_TYPE>::DefaultSetter))
+{}
+
+template <typename VALUE_TYPE>
+RWProperty<VALUE_TYPE>::RWProperty(GetterFunction getter_function,
+                                   SetterFunction setter_function)
+  : getter_function_(getter_function)
+  , setter_function_(setter_function)
+{}
+
+template <typename VALUE_TYPE>
+VALUE_TYPE RWProperty<VALUE_TYPE>::operator=(VALUE_TYPE const& value)
+{
+  return Set(value);
+}
+
+template <typename VALUE_TYPE>
+RWProperty<VALUE_TYPE>::operator VALUE_TYPE() const
+{
+  return getter_function_();
+}
+
+template <typename VALUE_TYPE>
+VALUE_TYPE RWProperty<VALUE_TYPE>::operator()() const
+{
+  return getter_function_();
+}
+
+template <typename VALUE_TYPE>
+VALUE_TYPE RWProperty<VALUE_TYPE>::operator()(VALUE_TYPE const& value)
+{
+  return Set(value);
+}
+
+template <typename VALUE_TYPE>
+VALUE_TYPE RWProperty<VALUE_TYPE>::Get() const
+{
+  return getter_function_();
+}
+
+template <typename VALUE_TYPE>
+VALUE_TYPE RWProperty<VALUE_TYPE>::Set(VALUE_TYPE const& value)
+{
+  if (setter_function_(value))
+  {
+    VALUE_TYPE new_value = getter_function_();
+    SignalBase::EmitChanged(new_value);
+    return new_value;
+  }
+  return getter_function_();
+}
+
+template <typename VALUE_TYPE>
+VALUE_TYPE RWProperty<VALUE_TYPE>::DefaultGetter() const
+{
+  return VALUE_TYPE();
+}
+
+template <typename VALUE_TYPE>
+bool RWProperty<VALUE_TYPE>::DefaultSetter(VALUE_TYPE const& value)
+{
+  return false;
+}
+
+template <typename VALUE_TYPE>
+void RWProperty<VALUE_TYPE>::SetSetterFunction(SetterFunction setter_function)
+{
+  setter_function_ = setter_function;
+}
+
+template <typename VALUE_TYPE>
+void RWProperty<VALUE_TYPE>::SetGetterFunction(GetterFunction getter_function)
+{
+  getter_function_ = getter_function;
+}
+
 
 // We need to provide a default constructor since we hide the copy ctor.
 inline Introspectable::Introspectable()
 {}
 
-inline void Introspectable::add_property(std::string const& name,
-                                         PropertyBase* property)
-{
-    // check to see if it exists and if it does barf horribly as we can't
-    // have two properties with the same name;
-    properties_[name] = property;
-}
-
-inline bool Introspectable::set_property(std::string const& name,
-                                         const char* value)
-{
-    PropertyContainer::iterator i = properties_.find(name);
-    if (i == properties_.end())
-        return false;
-    else
-        return i->second->set_value(value);
-}
-
-template <typename T>
-bool Introspectable::set_property(std::string const& name, T const& value)
-{
-    PropertyContainer::iterator i = properties_.find(name);
-    if (i == properties_.end())
-        return false;
-    else
-    {
-        return i->second->set_value(type::PropertyTrait<T>::to_string(value));
-    }
-}
-
-template <typename T>
-T Introspectable::get_property(std::string const& name, T* foo)
-{
-    PropertyContainer::iterator i = properties_.find(name);
-    if (i == properties_.end())
-        return T();
-
-    std::string s = i->second->get_serialized_value();
-    std::pair<T, bool> result = type::PropertyTrait<T>::from_string(s);
-    // If this is called with a template type that the property does not
-    // support nice conversion to, you'll get no error, but will get
-    // a default constructed T.  We could use an exception here.
-    return result.first;
-}
-
-
-template <typename T>
-Property<T>::Property(Introspectable* owner,
-                      std::string const& name)
-: Base()
-, name_(name)
-{
-    owner->add_property(name, this);
-}
-
-template <typename T>
-Property<T>::Property(Introspectable* owner,
-                      std::string const& name,
-                      T const& initial)
-: Base(initial)
-, name_(name)
-{
-    owner->add_property(name, this);
-}
-
-template <typename T>
-bool Property<T>::set_value(std::string const& serialized_form)
-{
-    std::pair<ValueType, bool> result = TraitType::from_string(serialized_form);
-    if (result.second) {
-      set(result.first);
-    }
-    return result.second;
-}
-
-template <typename T>
-std::string Property<T>::get_serialized_value() const
-{
-    return TraitType::to_string(Base::get());
-}
-
-template <typename T>
-typename Property<T>::ValueType const& Property<T>::operator=(typename Property<T>::ValueType const& value)
-{
-    set(value);
-    // There are no arguments to ‘get’ that depend on a template parameter,
-    // so we explicitly specify Base.
-    return Base::get();
+inline void Introspectable::AddProperty(std::string const& name,
+                                        PropertyBase* property)
+{
+  // check to see if it exists and if it does barf horribly as we can't
+  // have two properties with the same name;
+  properties_[name] = property;
+}
+
+inline bool Introspectable::SetProperty(std::string const& name,
+                                        const char* value)
+{
+  PropertyContainer::iterator i = properties_.find(name);
+  if (i == properties_.end())
+    return false;
+  else
+    return i->second->SetValue(value);
+}
+
+template <typename T>
+bool Introspectable::SetProperty(std::string const& name, T const& value)
+{
+  PropertyContainer::iterator i = properties_.find(name);
+  if (i == properties_.end())
+    return false;
+  else
+  {
+    return i->second->SetValue(type::PropertyTrait<T>::to_string(value));
+  }
+}
+
+template <typename T>
+T Introspectable::GetProperty(std::string const& name, T* foo)
+{
+  PropertyContainer::iterator i = properties_.find(name);
+  if (i == properties_.end())
+    return T();
+
+  std::string s = i->second->GetSerializedValue();
+  std::pair<T, bool> result = type::PropertyTrait<T>::from_string(s);
+  // If this is called with a template type that the property does not
+  // support nice conversion to, you'll get no error, but will get
+  // a default constructed T.  We could use an exception here.
+  return result.first;
+}
+
+
+template <typename T>
+SerializableProperty<T>::SerializableProperty(Introspectable* owner,
+                                              std::string const& name)
+  : Base()
+  , name_(name)
+{
+  owner->AddProperty(name, this);
+}
+
+template <typename T>
+SerializableProperty<T>::SerializableProperty(Introspectable* owner,
+                                              std::string const& name,
+                                              T const& initial)
+  : Base(initial)
+  , name_(name)
+{
+  owner->AddProperty(name, this);
+}
+
+template <typename T>
+bool SerializableProperty<T>::SetValue(std::string const& serialized_form)
+{
+  std::pair<ValueType, bool> result = TraitType::from_string(serialized_form);
+  if (result.second) {
+    Base::Set(result.first);
+  }
+  return result.second;
+}
+
+template <typename T>
+std::string SerializableProperty<T>::GetSerializedValue() const
+{
+  return TraitType::to_string(Base::Get());
+}
+
+template <typename T>
+T SerializableProperty<T>::operator=(T const& value)
+{
+  Base::Set(value);
+  // There are no arguments to ‘get’ that depend on a template parameter,
+  // so we explicitly specify Base.
+  return Base::Get();
 }
 
 

=== modified file 'NuxCore/Property.h'
--- NuxCore/Property.h	2011-06-27 14:18:42 +0000
+++ NuxCore/Property.h	2011-07-11 02:16:37 +0000
@@ -24,57 +24,174 @@
 
 #include "PropertyTraits.h"
 
+#include <string>
 #include <map>
 #include <sigc++/signal.h>
 
-
+/**
+ * Much of this property work is based on the work by Lois Goldthwaite,
+ * SC22/WG21/N1615=04-0055 - C++ Properties -- a Library Solution
+ *
+ * The basic ideas were extended to add update notifications, and
+ * serialisation and introspection.
+ */
 namespace nux {
 
-// TODO:
-//  object serialisation
 
 template <typename VALUE_TYPE>
-class ConnectableProperty
+class PropertyChangedSignal
 {
 public:
-  typedef typename type::PropertyTrait<VALUE_TYPE> TraitType;
-  typedef typename TraitType::ValueType ValueType;
-
-  ConnectableProperty();
-  ConnectableProperty(VALUE_TYPE const& initial);
-
-  VALUE_TYPE const& operator=(VALUE_TYPE const& value);
-  operator VALUE_TYPE const & () const;
-
-  // function call access
-  VALUE_TYPE const& operator()() const;
-  void operator()(VALUE_TYPE const& value);
-
-  // get and set access
-  VALUE_TYPE const& get() const;
-  void set(VALUE_TYPE const& value);
+  PropertyChangedSignal();
 
   sigc::signal<void, VALUE_TYPE const&> changed;
 
-  void disable_notifications();
-  void enable_notifications();
+  void DisableNotifications();
+  void EnableNotifications();
+
+  void EmitChanged(VALUE_TYPE const& new_value);
+
+private:
+  bool notify_;
+};
+
+/**
+ * A read/write property that stores the value type.
+ *
+ * The default setter emits the changed event if and only if the value
+ * changes.  A custom setter can be provided by passing in a setter function
+ * using sigc::mem_fun or sigc::ptr_fun.
+ */
+template <typename VALUE_TYPE>
+class Property : public PropertyChangedSignal<VALUE_TYPE>
+{
+public:
+  typedef PropertyChangedSignal<VALUE_TYPE> SignalBase;
+  typedef sigc::slot<bool, VALUE_TYPE&, VALUE_TYPE const&> SetterFunction;
+
+  Property();
+  explicit Property(VALUE_TYPE const& initial);
+  Property(VALUE_TYPE const& initial, SetterFunction setter_function);
+
+  VALUE_TYPE operator=(VALUE_TYPE const& value);
+  operator VALUE_TYPE() const;
+
+  // function call access
+  VALUE_TYPE operator()() const;
+  VALUE_TYPE operator()(VALUE_TYPE const& value);
+
+  // get and set access
+  VALUE_TYPE Get() const;
+  VALUE_TYPE Set(VALUE_TYPE const& value);
+
+  void SetSetterFunction(SetterFunction setter_function);
 
 private:
   // Properties themselves are not copyable.
-  ConnectableProperty(ConnectableProperty const&);
-  ConnectableProperty& operator=(ConnectableProperty const&);
+  Property(Property const&);
+  Property& operator=(Property const&);
+
+  bool DefaultSetter(VALUE_TYPE& target, VALUE_TYPE const& value);
 
 private:
   VALUE_TYPE value_;
-  bool notify_;
+  SetterFunction setter_function_;
+};
+
+// We could easily add a Write Only Property if and when we need one.
+
+/**
+ * A read only property that uses a function to get the value.
+ *
+ * The read only property does not have a changed signal.
+ *
+ * The default constructor creates a read only property that always returns
+ * the default constructed VALUE_TYPE.
+ */
+template <typename VALUE_TYPE>
+class ROProperty
+{
+public:
+  typedef sigc::slot<VALUE_TYPE> GetterFunction;
+
+  ROProperty();
+  explicit ROProperty(GetterFunction getter_function);
+
+  operator VALUE_TYPE() const;
+  VALUE_TYPE operator()() const;
+  VALUE_TYPE Get() const;
+
+  void SetGetterFunction(GetterFunction getter_function);
+
+private:
+  // ROProperties themselves are not copyable.
+  ROProperty(ROProperty const&);
+  ROProperty& operator=(ROProperty const&);
+
+  VALUE_TYPE DefaultGetter() const;
+
+private:
+  GetterFunction getter_function_;
+};
+
+/**
+ * A read/write property that uses a functions to get and set the value.
+ *
+ * The value type is not stored in the propery, but maintained by the setter
+ * and getter functions.
+ *
+ * A changed signal is emitted if the setter function specifies that the value
+ * has changed.
+ *
+ * The default setter does nothing and emits no signal, and the default getter
+ * returns a default constructed VALUE_TYPE.  The default getter and setter
+ * should be overridden through either the constructor args or through the
+ * SetGetterFunction / SetSetterFunction.
+ */
+template <typename VALUE_TYPE>
+class RWProperty : public PropertyChangedSignal<VALUE_TYPE>
+{
+public:
+  typedef PropertyChangedSignal<VALUE_TYPE> SignalBase;
+  typedef sigc::slot<bool, VALUE_TYPE const&> SetterFunction;
+  typedef sigc::slot<VALUE_TYPE> GetterFunction;
+
+  RWProperty();
+  RWProperty(GetterFunction getter_function, SetterFunction setter_function);
+
+  VALUE_TYPE operator=(VALUE_TYPE const& value);
+  operator VALUE_TYPE() const;
+
+  // function call access
+  VALUE_TYPE operator()() const;
+  VALUE_TYPE operator()(VALUE_TYPE const& value);
+
+  // get and set access
+  VALUE_TYPE Get() const;
+  VALUE_TYPE Set(VALUE_TYPE const& value);
+
+  void SetGetterFunction(GetterFunction getter_function);
+  void SetSetterFunction(SetterFunction setter_function);
+
+private:
+  // RWProperties themselves are not copyable.
+  RWProperty(RWProperty const&);
+  RWProperty& operator=(RWProperty const&);
+
+  VALUE_TYPE DefaultGetter() const;
+  bool DefaultSetter(VALUE_TYPE const& value);
+
+private:
+  GetterFunction getter_function_;
+  SetterFunction setter_function_;
 };
 
 
 class PropertyBase
 {
 public:
-  virtual bool set_value(std::string const& serialized_form) = 0;
-  virtual std::string get_serialized_value() const = 0;
+  virtual bool SetValue(std::string const& serialized_form) = 0;
+  virtual std::string GetSerializedValue() const = 0;
 };
 
 
@@ -86,15 +203,15 @@
 
   /// If the property was not able to be set with the value, the method
   /// returns false.
-  bool set_property(std::string const& name, const char* value);
-
-  template <typename T>
-  bool set_property(std::string const& name, T const& value);
-
-  template <typename T>
-  T get_property(std::string const& name, T* foo = 0);
-
-  void add_property(std::string const& name, PropertyBase* property);
+  bool SetProperty(std::string const& name, const char* value);
+
+  template <typename T>
+  bool SetProperty(std::string const& name, T const& value);
+
+  template <typename T>
+  T GetProperty(std::string const& name, T* foo = 0);
+
+  void AddProperty(std::string const& name, PropertyBase* property);
 
 private:
   // Introspectable objects are not copyable.
@@ -107,22 +224,25 @@
 };
 
 
-template <typename T>
-class Property : public ConnectableProperty<T>, public PropertyBase
+template <typename VALUE_TYPE>
+class SerializableProperty : public Property<VALUE_TYPE>, public PropertyBase
 {
 public:
-  typedef ConnectableProperty<T> Base;
-  typedef typename Base::ValueType ValueType;
-  typedef typename Base::TraitType TraitType;
-
-  Property(Introspectable* owner, std::string const& name);
-  Property(Introspectable* owner, std::string const& name, T const& initial);
-
-  virtual bool set_value(std::string const& serialized_form);
-  virtual std::string get_serialized_value() const;
+  typedef Property<VALUE_TYPE> Base;
+  typedef typename type::PropertyTrait<VALUE_TYPE> TraitType;
+  typedef typename TraitType::ValueType ValueType;
+
+  SerializableProperty(Introspectable* owner,
+                       std::string const& name);
+  SerializableProperty(Introspectable* owner,
+                       std::string const& name,
+                       VALUE_TYPE const& initial);
+
+  virtual bool SetValue(std::string const& serialized_form);
+  virtual std::string GetSerializedValue() const;
 
   // Operator assignment is not inherited nicely, so redeclare it here.
-  ValueType const& operator=(ValueType const& value);
+  VALUE_TYPE operator=(VALUE_TYPE const& value);
 
 private:
   std::string name_;

=== modified file 'tests/test_properties.cpp'
--- tests/test_properties.cpp	2011-06-27 14:18:42 +0000
+++ tests/test_properties.cpp	2011-07-11 02:16:37 +0000
@@ -1,11 +1,14 @@
 #include "NuxCore/Property.h"
 
+#include <boost/scoped_ptr.hpp>
 #include <sigc++/trackable.h>
 
-#include <gtest/gtest.h>
+#include <gmock/gmock.h>
 #include <vector>
 #include <stdexcept>
 
+using namespace testing;
+
 namespace {
 
 template <typename T>
@@ -98,62 +101,327 @@
 }
 
 
-TEST(TestConnectableProperty, TestConstruction) {
-  nux::ConnectableProperty<std::string> string_prop;
-  EXPECT_EQ("", string_prop());
-  EXPECT_EQ("", string_prop.get());
-  EXPECT_EQ("", static_cast<std::string>(string_prop));
-  nux::ConnectableProperty<std::string> string_prop_val("hello");
-  EXPECT_EQ("hello", string_prop_val());
-  EXPECT_EQ("hello", string_prop_val.get());
-  EXPECT_EQ("hello", static_cast<std::string>(string_prop_val));
-}
-
 template <typename T>
 struct ChangeRecorder : sigc::trackable
 {
+  typedef sigc::slot<void, T const&> Listener;
+
+  Listener listener()
+  {
+    return sigc::mem_fun(this, &ChangeRecorder<T>::value_changed);
+  }
+
   void value_changed(T const& value)
     {
       changed_values.push_back(value);
     }
   typedef std::vector<T> ChangedValues;
   ChangedValues changed_values;
+
+  int size() const { return changed_values.size(); }
+  T last() const { return *changed_values.rbegin(); }
 };
 
-TEST(TestConnectableProperty, TestAssignmentNotification) {
-  nux::ConnectableProperty<std::string> string_prop;
+
+TEST(TestProperty, TestDefaultConstructor) {
+  nux::Property<std::string> string_prop;
+  // Need either an assignment or static cast to check the operator VALUE_TYPE
+  // due to google-mock's template matching.
+  std::string value = string_prop;
+  EXPECT_THAT(value, Eq(""));
+  EXPECT_THAT(string_prop.Get(), Eq(""));
+  EXPECT_THAT(string_prop(), Eq(""));
+}
+
+TEST(TestProperty, TestValueExplicitConstructor) {
+  nux::Property<std::string> string_prop("Hello world!");
+  // Need either an assignment or static cast to check the operator VALUE_TYPE
+  // due to google-mock's template matching.
+  std::string value = string_prop;
+  EXPECT_THAT(value, Eq("Hello world!"));
+  EXPECT_THAT(string_prop.Get(), Eq("Hello world!"));
+  EXPECT_THAT(string_prop(), Eq("Hello world!"));
+}
+
+TEST(TestProperty, TestAssignment) {
+  nux::Property<std::string> string_prop;
+  // Need either an assignment or static cast to check the operator VALUE_TYPE
+  // due to google-mock's template matching.
+  string_prop = "Assignment operator";
+  std::string value = string_prop;
+  EXPECT_THAT(value, Eq("Assignment operator"));
+  EXPECT_THAT(string_prop.Get(), Eq("Assignment operator"));
+  EXPECT_THAT(string_prop(), Eq("Assignment operator"));
+
+  string_prop.Set("Set method");
+  value = string_prop;
+  EXPECT_THAT(value, Eq("Set method"));
+  EXPECT_THAT(string_prop.Get(), Eq("Set method"));
+  EXPECT_THAT(string_prop(), Eq("Set method"));
+
+  string_prop("Function call assignment");
+  value = string_prop;
+  EXPECT_THAT(value, Eq("Function call assignment"));
+  EXPECT_THAT(string_prop.Get(), Eq("Function call assignment"));
+  EXPECT_THAT(string_prop(), Eq("Function call assignment"));
+}
+
+TEST(TestProperty, TestChanged) {
+  nux::Property<std::string> string_prop;
   ChangeRecorder<std::string> recorder;
-  string_prop.changed.connect(
-    sigc::mem_fun(recorder, &ChangeRecorder<std::string>::value_changed));
+  string_prop.changed.connect(recorder.listener());
+
   string_prop = "Hello world" ;
-  EXPECT_EQ(1, recorder.changed_values.size());
-  EXPECT_EQ("Hello world", recorder.changed_values[0]);
+  EXPECT_THAT(1, Eq(recorder.size()));
+  EXPECT_THAT("Hello world", Eq(recorder.last()));
   // No notification if not changed.
   string_prop = std::string("Hello world");
-  EXPECT_EQ(1, recorder.changed_values.size());
+  EXPECT_THAT(1, Eq(recorder.size()));
 }
 
-TEST(TestConnectableProperty, TestEnableAndDisableNotification) {
-  nux::ConnectableProperty<std::string> string_prop;
+TEST(TestProperty, TestEnableAndDisableNotifications) {
+  nux::Property<std::string> string_prop;
   ChangeRecorder<std::string> recorder;
-  string_prop.changed.connect(
-    sigc::mem_fun(recorder, &ChangeRecorder<std::string>::value_changed));
-  string_prop.disable_notifications();
+  string_prop.changed.connect(recorder.listener());
+
+  string_prop.DisableNotifications();
   string_prop = "Hello world" ;
-  EXPECT_EQ(0, recorder.changed_values.size());
-  string_prop.enable_notifications();
+  EXPECT_THAT(0, Eq(recorder.size()));
+
+  string_prop.EnableNotifications();
   // No notification if not changed.
   string_prop = "Hello world" ;
-  EXPECT_EQ(0, recorder.changed_values.size());
+  EXPECT_THAT(0, Eq(recorder.size()));
 
   string_prop = "New value" ;
-  EXPECT_EQ(1, recorder.changed_values.size());
-  EXPECT_EQ("New value", recorder.changed_values[0]);
-
-  nux::ConnectableProperty<TestEnum> enum_prop;
-  // This fails to compile.
-  // nux::ConnectableProperty<TestClass> class_prop;
-}
+  EXPECT_THAT(1, Eq(recorder.size()));
+  EXPECT_THAT("New value", Eq(recorder.last()));
+}
+
+bool string_prefix(std::string& target, std::string const& value)
+{
+  bool changed = false;
+  std::string prefixed("prefix-" + value);
+  if (target != prefixed)
+  {
+    target = prefixed;
+    changed = true;
+  }
+  return changed;
+}
+
+TEST(TestProperty, TestSetterConstructor) {
+  nux::Property<std::string> string_prop("", sigc::ptr_fun(&string_prefix));
+
+  string_prop = "foo";
+  // Need either an assignment or static cast to check the operator VALUE_TYPE
+  // due to google-mock's template matching.
+  std::string value = string_prop;
+  EXPECT_THAT(value, Eq("prefix-foo"));
+  EXPECT_THAT(string_prop.Get(), Eq("prefix-foo"));
+  EXPECT_THAT(string_prop(), Eq("prefix-foo"));
+}
+
+class FloatClamp
+{
+public:
+  FloatClamp(float min, float max)
+    : min_(min), max_(max)
+    {
+    }
+  bool Set(float& target, float const& value)
+    {
+      bool changed = false;
+      float new_val = std::min(max_, std::max(min_, value));
+      if (target != new_val) {
+        target = new_val;
+        changed = true;
+      }
+      return changed;
+    }
+private:
+  float min_;
+  float max_;
+};
+
+TEST(TestProperty, TestCustomSetterFunction) {
+  nux::Property<float> float_prop;
+  FloatClamp clamp(0, 1);
+  float_prop.SetSetterFunction(sigc::mem_fun(&clamp, &FloatClamp::Set));
+  ChangeRecorder<float> recorder;
+  float_prop.changed.connect(recorder.listener());
+
+  // Since the default value for a float is zero, and we clamp at zero,
+  // setting to a negative value will result in setting to zero, which will
+  // not signal a changed event.
+  float_prop = -2;
+  EXPECT_THAT(float_prop(), Eq(0));
+  EXPECT_THAT(0, Eq(recorder.size()));
+
+  float_prop = 0.5;
+  EXPECT_THAT(float_prop(), Eq(0.5));
+  EXPECT_THAT(1, Eq(recorder.size()));
+  EXPECT_THAT(0.5, Eq(recorder.last()));
+
+  float_prop = 4;
+  EXPECT_THAT(float_prop(), Eq(1));
+  EXPECT_THAT(2, Eq(recorder.size()));
+  EXPECT_THAT(1, Eq(recorder.last()));
+}
+
+TEST(TestROProperty, TestDefaultConstructor) {
+  nux::ROProperty<int> int_prop;
+  int value = int_prop;
+  EXPECT_THAT(value, Eq(0));
+  EXPECT_THAT(int_prop(), Eq(0));
+  EXPECT_THAT(int_prop.Get(), Eq(0));
+
+  nux::ROProperty<std::string> string_prop;
+  std::string svalue = string_prop;
+  EXPECT_THAT(svalue, Eq(""));
+  EXPECT_THAT(string_prop(), Eq(""));
+  EXPECT_THAT(string_prop.Get(), Eq(""));
+}
+
+int simple_int_result()
+{
+  return 42;
+}
+
+TEST(TestROProperty, TestGetterConstructor) {
+  nux::ROProperty<int> int_prop(sigc::ptr_fun(&simple_int_result));
+  int value = int_prop;
+  EXPECT_THAT(value, Eq(42));
+  EXPECT_THAT(int_prop(), Eq(42));
+  EXPECT_THAT(int_prop.Get(), Eq(42));
+}
+
+class Incrementer
+{
+public:
+  Incrementer() : value_(0) {}
+  int value() { return ++value_; }
+private:
+  int value_;
+};
+
+TEST(TestROProperty, TestSetGetter) {
+  nux::ROProperty<int> int_prop;
+  Incrementer incrementer;
+  int_prop.SetGetterFunction(sigc::mem_fun(&incrementer, &Incrementer::value));
+
+  int value = int_prop;
+  EXPECT_THAT(value, Eq(1));
+  EXPECT_THAT(int_prop(), Eq(2));
+  EXPECT_THAT(int_prop.Get(), Eq(3));
+}
+
+
+TEST(TestRWProperty, TestDefaultConstructor) {
+  nux::RWProperty<int> int_prop;
+  ChangeRecorder<int> recorder;
+  int_prop.changed.connect(recorder.listener());
+
+  int_prop = 42;
+  int value = int_prop;
+  EXPECT_THAT(value, Eq(0));
+  EXPECT_THAT(int_prop(), Eq(0));
+  EXPECT_THAT(int_prop.Get(), Eq(0));
+  EXPECT_THAT(recorder.size(), Eq(0));
+}
+
+bool is_even(int const& value)
+{
+  return value % 2 == 0;
+}
+
+
+TEST(TestRWProperty, TestFunctionConstructor) {
+  // This is a somewhat convoluted example.  The setter emits if the value is
+  // even, but the value being emitted is controlled by the incrementer.
+  Incrementer incrementer;
+  nux::RWProperty<int> int_prop(sigc::mem_fun(&incrementer, &Incrementer::value),
+                                sigc::ptr_fun(&is_even));
+  ChangeRecorder<int> recorder;
+  int_prop.changed.connect(recorder.listener());
+
+  int_prop = 42;
+  EXPECT_THAT(recorder.size(), Eq(1));
+  EXPECT_THAT(recorder.last(), Eq(1));
+
+  // Catch the return value of the assignment.  The getter is called.
+  int assign_result = int_prop = 13;
+  EXPECT_THAT(recorder.size(), Eq(1));
+  EXPECT_THAT(assign_result, Eq(2));
+
+  // each access increments the value.
+  int value = int_prop;
+  EXPECT_THAT(value, Eq(3));
+  EXPECT_THAT(int_prop(), Eq(4));
+  EXPECT_THAT(int_prop.Get(), Eq(5));
+}
+
+// This bit would normally be in the header file.
+class HiddenImpl
+{
+public:
+  HiddenImpl();
+
+  nux::RWProperty<std::string> name;
+private:
+  class Impl;
+  boost::scoped_ptr<Impl> pimpl;
+};
+
+// This bit is in the implementation file.
+class HiddenImpl::Impl
+{
+public:
+  bool set_name(std::string const& name) {
+    bool changed = false;
+    std::string new_name("Impl::" + name);
+    if (name_ != new_name) {
+      name_ = new_name;
+      changed = true;
+    }
+    return changed;
+  }
+  std::string get_name() const {
+    return name_;
+  }
+
+private:
+  std::string name_;
+};
+
+HiddenImpl::HiddenImpl()
+  : pimpl(new Impl())
+{
+  name.SetSetterFunction(sigc::mem_fun(pimpl.get(), &HiddenImpl::Impl::set_name));
+  name.SetGetterFunction(sigc::mem_fun(pimpl.get(), &HiddenImpl::Impl::get_name));
+}
+
+
+TEST(TestRWProperty, TestPimplClassExample) {
+  HiddenImpl hidden;
+  ChangeRecorder<std::string> recorder;
+  hidden.name.changed.connect(recorder.listener());
+
+  hidden.name = "NewName";
+  EXPECT_THAT(recorder.size(), Eq(1));
+  EXPECT_THAT(recorder.last(), Eq("Impl::NewName"));
+
+  // Since the name is updated before comparison, no event emitted.
+  hidden.name = "NewName";
+  EXPECT_THAT(recorder.size(), Eq(1));
+
+  std::string value = hidden.name;
+  EXPECT_THAT(value, Eq("Impl::NewName"));
+  EXPECT_THAT(hidden.name(), Eq("Impl::NewName"));
+  EXPECT_THAT(hidden.name.Get(), Eq("Impl::NewName"));
+}
+
+
 
 struct TestProperties : nux::Introspectable
 {
@@ -162,15 +430,14 @@
     , index(this, "index")
     {}
 
-  nux::Property<std::string> name;
-  nux::Property<int> index;
+  nux::SerializableProperty<std::string> name;
+  nux::SerializableProperty<int> index;
 };
 
 TEST(TestIntrospectableProperty, TestSimplePropertyAccess) {
   TestProperties props;
   ChangeRecorder<std::string> recorder;
-  props.name.changed.connect(
-    sigc::mem_fun(recorder, &ChangeRecorder<std::string>::value_changed));
+  props.name.changed.connect(recorder.listener());
   EXPECT_EQ("", props.name());
   EXPECT_EQ(0, props.index());
   props.name = "Testing";
@@ -178,10 +445,10 @@
   EXPECT_EQ("Testing", props.name());
   props.name("New Value");
   EXPECT_EQ("New Value", props.name());
-  props.name.set("Another");
+  props.name.Set("Another");
   EXPECT_EQ("Another", props.name());
 
-  EXPECT_EQ(3, recorder.changed_values.size());
+  EXPECT_EQ(3, recorder.size());
   EXPECT_EQ("Testing", recorder.changed_values[0]);
   EXPECT_EQ("New Value", recorder.changed_values[1]);
   EXPECT_EQ("Another", recorder.changed_values[2]);
@@ -191,52 +458,49 @@
   TestProperties props;
   ChangeRecorder<std::string> name_recorder;
   ChangeRecorder<int> index_recorder;
-  props.name.changed.connect(
-    sigc::mem_fun(name_recorder, &ChangeRecorder<std::string>::value_changed));
-  props.index.changed.connect(
-    sigc::mem_fun(index_recorder, &ChangeRecorder<int>::value_changed));
+  props.name.changed.connect(name_recorder.listener());
+  props.index.changed.connect(index_recorder.listener());
 
   props.name = "Testing";
   props.index = 5;
-  EXPECT_EQ("Testing", props.get_property<std::string>("name"));
-  EXPECT_EQ("5", props.get_property<std::string>("index"));
-  EXPECT_EQ(5, props.get_property<int>("index"));
+  EXPECT_EQ("Testing", props.GetProperty<std::string>("name"));
+  EXPECT_EQ("5", props.GetProperty<std::string>("index"));
+  EXPECT_EQ(5, props.GetProperty<int>("index"));
 
-  bool assigned = props.set_property("name", "New value");
+  bool assigned = props.SetProperty("name", "New value");
   EXPECT_TRUE(assigned);
   EXPECT_EQ("New value", props.name());
-  EXPECT_EQ("New value", props.get_property<std::string>("name"));
+  EXPECT_EQ("New value", props.GetProperty<std::string>("name"));
   // A little dangreous, but legal.
-  EXPECT_EQ(0, props.get_property<int>("name"));
+  EXPECT_EQ(0, props.GetProperty<int>("name"));
 
-  assigned = props.set_property("name", 42);
+  assigned = props.SetProperty("name", 42);
   EXPECT_TRUE(assigned);
   EXPECT_EQ("42", props.name());
-  EXPECT_EQ("42", props.get_property<std::string>("name"));
+  EXPECT_EQ("42", props.GetProperty<std::string>("name"));
   // A little dangreous, but legal.
-  EXPECT_EQ(42, props.get_property<int>("name"));
+  EXPECT_EQ(42, props.GetProperty<int>("name"));
 
-  assigned = props.set_property("index", 42);
+  assigned = props.SetProperty("index", 42);
   EXPECT_TRUE(assigned);
   EXPECT_EQ(42, props.index());
-  EXPECT_EQ("42", props.get_property<std::string>("index"));
-  EXPECT_EQ(42, props.get_property<int>("index"));
+  EXPECT_EQ("42", props.GetProperty<std::string>("index"));
+  EXPECT_EQ(42, props.GetProperty<int>("index"));
 
-  assigned = props.set_property("index", "hello");
+  assigned = props.SetProperty("index", "hello");
   EXPECT_FALSE(assigned);
   EXPECT_EQ(42, props.index());
-  EXPECT_EQ("42", props.get_property<std::string>("index"));
-  EXPECT_EQ(42, props.get_property<int>("index"));
+  EXPECT_EQ("42", props.GetProperty<std::string>("index"));
+  EXPECT_EQ(42, props.GetProperty<int>("index"));
 
   // Gettin a non-existant property returns a default constructed instance.
-  std::string surname = props.get_property<std::string>("surname");
+  std::string surname = props.GetProperty<std::string>("surname");
   EXPECT_EQ("", surname);
-  int foo = props.get_property<int>("foo");
+  int foo = props.GetProperty<int>("foo");
   EXPECT_EQ(0, foo);
 
-  assigned = props.set_property("non-existant", "hello");
+  assigned = props.SetProperty("non-existant", "hello");
   EXPECT_FALSE(assigned);
-
 }