2011/5/9 Enrico Schröder <enni.schroeder@xxxxxxxxx>: > 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 GimpUnitAdjustment. * 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: http://git.gnome.org/browse/gimp/commit/?id=9fefa22efe70e484fc7c92708ed8efe023e4d219 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, Martin -- My GIMP Blog: http://www.chromecode.com/ "GIMP 2.8 schedule on tasktaste.com" _______________________________________________ Gimp-developer mailing list Gimp-developer@xxxxxxxxxxxxxxxxxxxxxx https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
