Community
Participate
Working Groups
Build Identifier: Eclipse 3.7 20110615-0604, Graphiti 0.8.0.v20110607-1252 Setters can lead to missuse, e.g. create font and changing it afterwards with the setters and calling this logic multiple times in the code leads to duplicate font objects in the diagram. In contrast to that calling the factory method multiple times with the same values will return the already existing font object, see forum thread for background: http://www.eclipse.org/forums/index.php/t/230825/ Reproducible: Always
Having the setters at the managed objects (fonts and colors) is indeed errorprone. It can lead to completely misleading the instance management done by Graphiti. Here's a short explanation for sake of completeness: - While adding a shape to the diagram a text field is created using the default font (createDefaultText method) - The font of that text field is changed again (still in the add) to let's say size 10 (default is 8) - Adding a second shape like the first one will create a second default font instance, which again will be changed to size 10 --> result is having 2 font instances with the same attributes inside a diagram --> each add operation for that shape will add one more The same can happen with colors.
I have removed the setters for all attributes of the Graphiti colors and fonts. Changes are checked in locally. - Removed attribute setter for font: name, size, italic, bold - Adapted the creation of fonts in the Graphiti framework - Adapted the example coding - Added conveniance methods for managing default fonts and tests for those methods - Removed attribute setters for color: red, green, blue - Adapted the creation of colors in the Graphiti framework - Added some JavaDoc and docu in the tutorial where font and color management is used for the first time
Pushed to Eclipse Git repository. Last change is: commit 0c8cb70b848e230cfe4854147fd16d1677d6c3d5 Author: mwenz <michael.wenz@sap.com> 2011-09-06 15:25:57 Committer: mwenz <michael.wenz@sap.com> 2011-09-06 15:25:57 Parent: b118d179ec315a04117f340f22d8ff9eb49e9d3d (Bug 355347 - Added some documentation) Branches: origin/master, master
This change means that all users of the setters for the attributes of fonts and colors need to adapt their coding. The creation of fonts e.g. must follow this pattern: IGaService gaService = Graphiti.getGaService(); Text text = gaService.createText(parentShape, "Text"); text.setFont(gaService.manageDefaultFont(getDiagram(), false, true)); or to set any other font: IGaService gaService = Graphiti.getGaService(); Text text = gaService.createText(parentShape, "Text"); text.setFont(gaService.manageFont(getDiagram(), "Comic Sans MS", 14););
(In reply to comment #4) > This change means that all users of the setters for the attributes of fonts and > colors need to adapt their coding. The creation of fonts e.g. must follow this > pattern: > IGaService gaService = Graphiti.getGaService(); > Text text = gaService.createText(parentShape, "Text"); > text.setFont(gaService.manageDefaultFont(getDiagram(), false, true)); > > or to set any other font: > IGaService gaService = Graphiti.getGaService(); > Text text = gaService.createText(parentShape, "Text"); > text.setFont(gaService.manageFont(getDiagram(), "Comic Sans MS", 14);); I would recommend to put the font inside the createText method. I once had switched from DefaultText to Text and have forgotten to set the font causing in NPE when trying to calculate the text dimension. I would even recommand to consider replacing all setters in favor of using something like a builder pattern. The builder can hold some default values so even without specifing any additional properties you create a valid object: I have tried something like that: https://github.com/joergreichert/Permet/blob/master/org.eclipse.xtext.example.fowlerdsl.diagram/src/org/eclipse/xtext/example/fowlerdsl/diagram/features/add/AddStateFeature.java
(In reply to comment #5) > I would recommend to put the font inside the createText method. I once had > switched from DefaultText to Text and have forgotten to set the font causing > an NPE when trying to calculate the text dimension. Good point. I added creator methods for both Text and MultiText that take the description of a font managed inside a given diagram. Commited and pushed to Eclipse: commit d47e58a6af674fba8c57113c7fbae2f8108fdd39 Author: mwenz <michael.wenz@sap.com> 2011-09-07 10:02:59 Committer: mwenz <michael.wenz@sap.com> 2011-09-07 10:02:59 Parent: 0c8cb70b848e230cfe4854147fd16d1677d6c3d5 (Bug 355347 - Updated file headers) Branches: origin/master, master > I would even recommand to consider replacing all setters in favor of using > something like a builder pattern. The builder can hold some default values so > even without specifing any additional properties you create a valid object: > I have tried something like that: > >https://github.com/joergreichert/Permet/blob/master/org.eclipse.xtext.example.f>owlerdsl.diagram/src/org/eclipse/xtext/example/fowlerdsl/diagram/features/add/A>ddStateFeature.java Also a good idea, but I would suggest to split that off into a seperate enhancement bugzilla; I have created https://bugs.eclipse.org/bugs/show_bug.cgi?id=356893 for that. It seems that you already have something like that. Would you be interested in contributing?
I downported the relevant parts of this fix to Indigo SR1: - Set setter methods in Font and Color are deprecated now - The usages in the framework and examples are adapted and use the recommended way now Checked in and pushed to Eclipe: commit 34832ec72f8f1a667db0f5b30b595f73f620affe Author: mwenz <michael.wenz@sap.com> 2011-09-05 17:02:18 Committer: mwenz <michael.wenz@sap.com> 2011-09-07 10:29:03 Parent: 2371bdfb67216c2c09b1325860e456046263e64a (Updated news¬eworthy for Indigo SR1) Child: 3c289a3833cff6744b867773f750d71abcd19dc8 (Bug 355347 - Adapt creation of Colors via Graphiti API) Branches: origin/b0_8_x, b0_8_x commit 3c289a3833cff6744b867773f750d71abcd19dc8 Author: mwenz <michael.wenz@sap.com> 2011-09-06 13:58:13 Committer: mwenz <michael.wenz@sap.com> 2011-09-07 10:49:50 Parent: 34832ec72f8f1a667db0f5b30b595f73f620affe (Bug 355347 - Adapt creation of Fonts via Graphiti API) Child: d673199fbe42ffd7e15a668c5e2324e6094cdb4c (Bug 355347 - Deprecated setter methods for attributes of Font and Color and adapted framework and examples usages) Branches: origin/b0_8_x, b0_8_x commit d673199fbe42ffd7e15a668c5e2324e6094cdb4c Author: mwenz <michael.wenz@sap.com> 2011-09-07 10:57:11 Committer: mwenz <michael.wenz@sap.com> 2011-09-07 10:57:11 Parent: 3c289a3833cff6744b867773f750d71abcd19dc8 (Bug 355347 - Adapt creation of Colors via Graphiti API) Branches: origin/b0_8_x, b0_8_x
Bookkeeping: Set target release: 0.8.1
Part of Graphiti 0.9.0 (Eclipse Juno)
