Version: (using KDE Devel) Installed from: Compiled sources 1. The distinction between UMLApp and UMLDoc is unclear. For example, both UMLApp and UMLDoc have a method getCurrentView() which however do not necessarily return the same object?! IMHO UMLDoc should be merged into UMLApp. 2. It is not clear why signals are used for updating the UMLListView. IMHO direct calls to UMLListView methods would make more sense.
SVN commit 442486 by okellogg: CCBUG:107347 - Address the second part of the PR about gratuitous use of signals for updating the list view. This change reveals the need for further design cleanups. M +14 -9 classifier.cpp M +0 -6 classifier.h M +0 -8 umllistview.cpp M +0 -12 umlobject.h M +4 -1 umlwidget.cpp
At http://www.geeksoc.org/~jr/umbrello/uml-devel/9363.html, Daniel Calviño Sánchez <danxuliu_at_gmail.com> wrote: > I'm adding an option to the "Export all views" action > (http://bugs.kde.org/58809) and I have some questions about code > style. The option I'm adding is keep the tree structure used to store > the views in the document in the target directory, so if you have your > diagrams in folders created by you in the document, those folders are > created when exporting (Logical view, use case view and so on aren't > created). Interesting. Sounds like there might be some overlap with http://bugs.kde.org/87252 > In order to add that option, I've also refactored a bit the > ExportViewAction class and the exportImage() method in UMLView. I've > made a class, UMLViewImageExporter, that contains the methods that do > "all the work", and that class is used by other methods that take care > of asking the user where to save the image, the format to use, and so > on. > > The problem is that the non-interactive methods can fail due to > various things. So I thought in throwing exceptions and catch them > where they were called so an information message is shown to the user > if needed (for example, if the exporting was made through > command-line, the message box wouldn't be shown). > > But, after a search, I found that no exceptions are thrown in Umbrello > code (at least, I haven't found them). So, the question is, can I use > exceptions? Or maybe I should think in another way to report errors > (with return values, for example)? I mean, perhaps you don't use > exceptions because of compiler compatibility or something like that. True, Umbrello has not been using exceptions - mainly due to old-compiler concerns. (Should be less of an issue nowadays.) OTOH I'm not sure how Qt and KDE get along with exceptions. So to be on the safe side I'd still recommend return values instead of exceptions. > The other question is related to where put the interactive code. Right > now, the "Export view" action code is in UMLView class. The "Export > all views" action is in ExportViewAction class, which is a KAction > subclass. I think that the UMLView class code should be extracted to > another class, maybe another KAction subclass. True, UMLView has too much stuff lumped in; the same goes for UMLApp, and UMLDoc should be eliminated altogether (see discussion at top.) > Or maybe you prefer the UMLView code to stay there and move the > ExportViewAction code to UMLApp. Or maybe let it as it is now. But I > think that it'd be better if a more consistent approach is used than > an action for the "Export all views" and embedded code in UMLView for > "Export view". I'm renaming the bug title to "separation of concerns" exactly for this reason that we start thinking about the global objects in the program. For example, it may well be that ExportViewAction deserves its own, standalone global object (the same might be true for Undo/Redo and other entities.) It's not clear to me why UMLApp should need to be the container for all global objects; if somebody knows please explain.
Qt3 by default is compiled without exception support, so if you call a function in the Qt library which in turn calls a function in your code which throws an exception, then the exception will not propagate all the way up the call stack unless the Qt library is compiled with exception support. Qt3 apps and KDE3 apps in general do not throw exceptions because of memory bloat issues in the implementations of many compiler's exception handling code.
While working on a completely unrelated issue (wish 114547), all of a sudden I get this crash: [KCrash handler] #4 0x40ffdecb in QString::QString(QString const&) () from /usr/lib/qt3/lib/libqt-mt.so.3 #5 0x08136f50 in GeneralState (this=0xbfffeef0, _ctor_arg=@0x8457c8c) at uml.h:1001 #6 0x08136d88 in OptionState (this=0xbfffeef0, _ctor_arg=@0x8457c8c) at uml.h:1001 #7 0x08136a24 in UMLApp::getOptionState() (this=0x8457ab8) at uml.h:1001 #8 0x081c0c59 in UMLListView::setDocument(UMLDoc*) (this=0x84eaf90, d=0x84647b8) at /home/kellogg/kdesdk-3.5-branch/umbrello/umbrello/umllistview.cpp:850 #9 0x081a557a in UMLApp::initView() (this=0x8457ab8) at /home/kellogg/kdesdk-3.5-branch/umbrello/umbrello/uml.cpp:469 #10 0x081a1244 in UMLApp (this=0x8457ab8, name=0x0) at /home/kellogg/kdesdk-3.5-branch/umbrello/umbrello/uml.cpp:99 #11 0x0817c643 in main (argc=1, argv=0xbffff224) at main.cpp:101 #12 0x415158ae in __libc_start_main () from /lib/libc.so.6 How could this ever work? At #7 we're calling UMLApp::app()->getOptionState() at a time when the app() instance is not even finished being constructed. I will move the {set,get}OptionState() methods to Settings.
SVN commit 563149 by okellogg: Move all code import related files to a separate subdirectory, codeimport. Prepare for Pascal import and generator. ATM they are cheap copies from Ada, adaptation to Pascal is up next. Also move the {set,get}OptionState() methods from UMLApp to Settings because of a crash (that should have happened long ago...) CCBUG:114547 CCBUG:107347 M +1 -0 ChangeLog M +3 -10 umbrello/Makefile.am D umbrello/adaimport.cpp D umbrello/adaimport.h M +1 -1 umbrello/associationwidget.cpp M +2 -2 umbrello/classifier.cpp D umbrello/classimport.cpp D umbrello/classimport.h M +1 -1 umbrello/classparser/cpptree2uml.cpp M +2 -2 umbrello/codegenerators/Makefile.am M +7 -3 umbrello/codegenerators/codegenfactory.cpp A umbrello/codegenerators/pascalwriter.cpp [License: GPL (v2+)] A umbrello/codegenerators/pascalwriter.h [License: GPL (v2+)] A umbrello/codeimport (directory) A umbrello/codeimport/Makefile.am A umbrello/codeimport/adaimport.cpp umbrello/adaimport.cpp#562821 [License: GPL (v2+)] A umbrello/codeimport/adaimport.h umbrello/adaimport.h#562821 A umbrello/codeimport/classimport.cpp umbrello/classimport.cpp#562821 [License: GPL (v2+)] A umbrello/codeimport/classimport.h umbrello/classimport.h#562821 A umbrello/codeimport/cppimport.cpp umbrello/cppimport.cpp#562821 [License: GPL (v2+)] A umbrello/codeimport/cppimport.h umbrello/cppimport.h#562821 A umbrello/codeimport/idlimport.cpp umbrello/idlimport.cpp#562821 [POSSIBLY UNSAFE: popen] [License: GPL (v2+)] A umbrello/codeimport/idlimport.h umbrello/idlimport.h#562821 A umbrello/codeimport/import_utils.cpp umbrello/import_utils.cpp#562840 [License: GPL (v2+)] A umbrello/codeimport/import_utils.h umbrello/import_utils.h#562821 [License: GPL (v2+) GENERATED FILE] A umbrello/codeimport/javaimport.cpp umbrello/javaimport.cpp#562848 [License: GPL (v2+)] A umbrello/codeimport/javaimport.h umbrello/javaimport.h#562821 A umbrello/codeimport/nativeimportbase.cpp umbrello/nativeimportbase.cpp#562821 A umbrello/codeimport/nativeimportbase.h umbrello/nativeimportbase.h#562821 [License: GPL (v2+)] A umbrello/codeimport/pascalimport.cpp [License: GPL (v2+)] A umbrello/codeimport/pascalimport.h [License: GPL (v2+)] A umbrello/codeimport/pythonimport.cpp umbrello/pythonimport.cpp#562821 [License: GPL (v2+)] A umbrello/codeimport/pythonimport.h umbrello/pythonimport.h#562821 D umbrello/cppimport.cpp D umbrello/cppimport.h M +4 -7 umbrello/dialogs/umlattributedialog.cpp M +5 -8 umbrello/dialogs/umlwidgetcolorpage.cpp M +1 -1 umbrello/entity.cpp D umbrello/idlimport.cpp D umbrello/idlimport.h D umbrello/import_utils.cpp D umbrello/import_utils.h D umbrello/javaimport.cpp D umbrello/javaimport.h M +1 -1 umbrello/listpopupmenu.cpp M +4 -0 umbrello/model_utils.cpp D umbrello/nativeimportbase.cpp D umbrello/nativeimportbase.h A umbrello/optionstate.cpp [License: GPL (v2+)] M +7 -1 umbrello/optionstate.h M +1 -1 umbrello/petaltree2uml.cpp D umbrello/pythonimport.cpp D umbrello/pythonimport.h M +106 -95 umbrello/uml.cpp M +1 -10 umbrello/uml.h M +12 -12 umbrello/umldoc.cpp M +3 -4 umbrello/umllistview.cpp M +1 -0 umbrello/umlnamespace.h M +2 -2 umbrello/umlobject.cpp
SVN commit 577629 by okellogg: Remove the confusing UMLDoc::getCurrentView() method, class UMLApp is responsible for the current view. This addresses part of the PR: > 1. The distinction between UMLApp and UMLDoc is unclear. > For example, both UMLApp and UMLDoc have a method getCurrentView() > which however do not necessarily return the same object?! CCBUG:107347 M +1 -1 Makefile.am M +1 -1 aligntoolbar.cpp M +14 -16 clipboard/umlclipboard.cpp M +2 -2 clipboard/umldrag.cpp M +1 -1 codegenerator.cpp M +1 -1 codeimport/nativeimportbase.cpp M +4 -6 dialogs/classgenpage.cpp M +1 -1 dialogs/classpropdlg.cpp M +7 -9 dialogs/diagramprintpage.cpp D infowidget.cpp D infowidget.h M +2 -5 linkwidget.cpp M +27 -31 uml.cpp M +2 -9 uml.h M +29 -81 umldoc.cpp M +0 -11 umldoc.h M +11 -8 umllistview.cpp M +10 -185 umlview.cpp A widget_factory.cpp [License: GPL (v2+)] A widget_factory.h [License: GPL (v2+)] M +2 -5 widget_utils.cpp M +4 -7 worktoolbar.cpp
Is this bug done or are there any additional patches required ?
(In reply to Ralf Habacker from comment #7) > Is this bug done or are there any additional patches required ? Hmm, all in all I think the major issues have been addressed, except perhaps for the unclear distinction between UMLApp and UMLDoc.