2011/5/9 Enrico Schröder <enni.schroe...@gmail.com>:
> Hi everybody,
> I've come up with a updated (and more detailed) version of class and
> sequence diagrams for the new widget.
> http://enni.userpage.fu-berlin.de/GimpUnitEntry.pdf
> I tried to incorporate some of the comments. Note that all names are
> subject to change ;-)
> Our now called GimpUnitEntry will be derived from GtkSpinScale and use a
> subclass of GtkAdjustment to store its value including the unit
> (GimpUnitAdjustment). All synchronisation and live updating of
> associated UnitEntries will happen directly between their
> GimpUnitAdjustments, thus completely separating everything value-related
> from the gui and input. The GimpUnitEntry itself will be just
> responsible for display and parsing of entered terms. See use cases and
> class diagram for details.

Hi Enrico,

Good job! Very clear. Some comments, questions and design concerns:

* I think the class names are fine.

* The parsing should also be perfomed by a non-UI layer and not in
GimpUnitEntry so the parsing code can be used in e.g. a script
interpreter with no UI dependencies.

* Yes, try to reuse GtkAdjustment::value-changed. Only if that doesn't
work for some reason should you duplicate the signal in

* GimpUnitEntryTable shouldn't derive from GtkTable, because there is
no is-a relationship. To convince yourself of that: Can you always
replace a GtkTable with a GimpUnitEntryTable? Nope, so you should use
composition instead of inheritance.

* In case mitch didn't already tell you: The easiest way to create a
new class including all C GObject boilerplate is to copy an existing
class and do word-wize and case-preserving (like Emacs)
search-and-replace. For example, to create GimpUnitAdjustment, copy a
GObject inherited GimpDoubleWorded class and first replace 'double'
with 'unit', then 'worded' with 'adjustment'.

* I've recomended a test-case based development approach before, and
now that most of the code will have no UI-dependencies I recomend it
even more, as non-UI classes generally are very easy to unit-test. An
example of a commit that introduces a non-UI class along with
appropriate propotions of documentation and unit tests is this commit:
By writing unit tests along with your code, productivity goes up a lot
when you (or anyone in the lifetime of the project) refactor code,
because verifying the class still works takes seconds.

* Regarding the use case of entering "1 cm" and getting the result in
px, we could use an 'in' keyword like Google. Analogous to "100 SEK in
USD", we would have "1 cm in px". That is a future extension though,
don't spend time on that yet.

* Have you thought about how the image resolution comes into the picture?

* Regarding your get_unit() method, an interesting question is what
unit an expression like "10px + 1cm" has.

* Have you thought about how to connect GimpUnitEntry to the existing
GIMP code like the various props helper functions? (See how
GimpSizeEntry currently is used)

* I don't like how constraints currently are applied, in particular
that you require two instance of a constraint when there is only one
equation that shall hold. I don't think it is a good idea to make
GimpUnitAdjustment (or GimpUnitEntry for that matter) be aware of
constraints, because they can potentially be rather complex. It is
probably a better idea to take care of constraints one one
architectural level above GimpUnitAdjustment (they don't know about
constraints, constraints are just being applied to them).

* Regarding the contraint case, for a good design, consider how you
would support constraints between three numbers (e.g. a = b + c).

* Again, I would advice you to focus on the basics. Once the basic
functionality is in place, meaning it is good enough for inclusion in
a stable GIMP version, we can start working on constraints support.

* What program did you make those rather nice sequence diagrams in?

Best regards,

My GIMP Blog:
"GIMP 2.8 schedule on tasktaste.com"
Gimp-developer mailing list

Reply via email to