Code review comment for lp:~thumper/nux/properties

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) 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.
   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 :-)

« Back to merge proposal