Hi, I was just waiting for these kind of comments. >:-(
In short: 1. Yes, formatting in Eclipse sometimes happens by accident, it's too easy. Sorry, it must have slipped through. My fault. I know the rule. 2. Regarding sorting algo: we know that, so add a TODO to the code, there's no violation in doing so. There's no need to bring such kind of discussion to the dev list, add a comment to the issue. Please consider that this was a first patch by an interested developer, so find a nicer way to improve his contribution, ok? Thomas -------- Original-Nachricht -------- > Datum: Tue, 24 Mar 2009 20:49:15 -0400 > Von: Tom Morris <[email protected]> > An: [email protected] > Betreff: Re: [argouml-dev] Sorting generator languages (Re: svn commit: > r16941) > That patch appears to have a ton of unrelated white space changes > (which is an issue in and of itself) and I can't easily see where the > issue is, but I agree that no one should be implementing their own > sorting code these days. > > The interface is actually called Comparable (no 'I'), but other than > that, the approach sounds reasonable. > > Tom > > On Tue, Mar 24, 2009 at 6:52 PM, Christian López Espínola > <[email protected]> wrote: > > Hi Thomas, Christian, > > > > I find not safe writing our own sorting algorithm. What about making > > Language implement IComparable, and call Collections.sort? > > > > I find this as a good exercise for Christian to get started with our > > code review practices and coding standards. > > If you don't have the time needed for this, please decline ;-) > > If you need any help, you know that I am on IRC. > > > > Thanks for the patch! > > > > On Tue, Mar 24, 2009 at 7:30 PM, Thomas Neustupny <[email protected]> wrote: > >> Author: thn > >> Date: 2009-03-24 11:30:00-0700 > >> New Revision: 16941 > >> > >> Modified: > >> > trunk/src/argouml-app/src/org/argouml/uml/generator/ui/ClassGenerationDialog.java > >> > >> Log: > >> the patch from Christian Heinrich for issue 5743: > >> Patch for ordering language names alphabetically in "generate all > classes" window > >> > >> Modified: > trunk/src/argouml-app/src/org/argouml/uml/generator/ui/ClassGenerationDialog.java > >> Url: > http://argouml.tigris.org/source/browse/argouml/trunk/src/argouml-app/src/org/argouml/uml/generator/ui/ClassGenerationDialog.java?view=diff&pathrev=16941&r1=16940&r2=16941 > >> > ============================================================================== > >> --- > trunk/src/argouml-app/src/org/argouml/uml/generator/ui/ClassGenerationDialog.java > (original) > >> +++ > trunk/src/argouml-app/src/org/argouml/uml/generator/ui/ClassGenerationDialog.java > 2009-03-24 11:30:00-0700 > >> @@ -66,35 +66,35 @@ > >> /** > >> * The dialog that starts the generation of classes. > >> */ > >> -public class ClassGenerationDialog > >> - extends ArgoDialog > >> - implements ActionListener { > >> +public class ClassGenerationDialog extends ArgoDialog implements > ActionListener { > >> > >> private static final String SOURCE_LANGUAGE_TAG = "src_lang"; > >> - > >> + > >> /** > >> * Logger. > >> */ > >> - private static final Logger LOG = > >> - Logger.getLogger(ClassGenerationDialog.class); > >> + private static final Logger LOG = Logger > >> + .getLogger(ClassGenerationDialog.class); > >> > >> private TableModelClassChecks classTableModel; > >> + > >> private boolean isPathInModel; > >> + > >> private List<Language> languages; > >> > >> private JTable classTable; > >> + > >> private JComboBox outputDirectoryComboBox; > >> > >> /** > >> - * Used to select the next language column in case > >> - * the "Select All" button is pressed. > >> + * Used to select the next language column in case the "Select > All" button > >> + * is pressed. > >> */ > >> private int languageHistory; > >> > >> - > >> /** > >> * Constructor. > >> - * > >> + * > >> * @param nodes The UML elements, typically classifiers, to > generate. > >> */ > >> public ClassGenerationDialog(List<Object> nodes) { > >> @@ -103,16 +103,14 @@ > >> > >> /** > >> * Constructor. > >> - * > >> + * > >> * @param nodes The UML elements, typically classifiers, to > generate. > >> - * @param inModel <code>true</code> if the path is in the model. > >> - * TODO: Correct? > >> + * @param inModel <code>true</code> if the path is in the model. > TODO: > >> + * Correct? > >> */ > >> public ClassGenerationDialog(List<Object> nodes, boolean inModel) > { > >> - super( > >> - > Translator.localize("dialog.title.generate-classes"), > >> - Dialog.OK_CANCEL_OPTION, > >> - true); > >> + > super(Translator.localize("dialog.title.generate-classes"), > >> + Dialog.OK_CANCEL_OPTION, true); > >> isPathInModel = inModel; > >> > >> buildLanguages(); > >> @@ -138,7 +136,9 @@ > >> nameButton(selectAllButton, "button.select-all"); > >> selectAllButton.addActionListener(new ActionListener() { > >> /* > >> - * @see > java.awt.event.ActionListener#actionPerformed(java.awt.event.ActionEvent) > >> + * @see > >> + * > java.awt.event.ActionListener#actionPerformed(java.awt.event. > >> + * ActionEvent) > >> */ > >> public void actionPerformed(ActionEvent e) { > >> classTableModel.setAllChecks(true); > >> @@ -149,7 +149,9 @@ > >> nameButton(selectNoneButton, "button.select-none"); > >> selectNoneButton.addActionListener(new ActionListener() { > >> /* > >> - * @see > java.awt.event.ActionListener#actionPerformed(java.awt.event.ActionEvent) > >> + * @see > >> + * > java.awt.event.ActionListener#actionPerformed(java.awt.event. > >> + * ActionEvent) > >> */ > >> public void actionPerformed(ActionEvent e) { > >> classTableModel.setAllChecks(false); > >> @@ -165,22 +167,23 @@ > >> selectPanel.add(selectButtons); > >> > >> JPanel centerPanel = new JPanel(new BorderLayout(0, 2)); > >> - centerPanel.add(new JLabel(Translator.localize( > >> - "label.available-classes")), BorderLayout.NORTH); > >> + centerPanel.add(new JLabel(Translator > >> + .localize("label.available-classes")), > BorderLayout.NORTH); > >> centerPanel.add(new JScrollPane(classTable), > BorderLayout.CENTER); > >> centerPanel.add(selectPanel, BorderLayout.SOUTH); > >> contentPanel.add(centerPanel, BorderLayout.CENTER); > >> > >> // Output Directory > >> - outputDirectoryComboBox = > >> - new > JComboBox(getClasspathEntries().toArray()); > >> + outputDirectoryComboBox = new > JComboBox(getClasspathEntries().toArray()); > >> > >> JButton browseButton = new JButton(); > >> nameButton(browseButton, "button.browse"); > >> browseButton.setText(browseButton.getText() + "..."); > >> browseButton.addActionListener(new ActionListener() { > >> /* > >> - * @see > java.awt.event.ActionListener#actionPerformed(java.awt.event.ActionEvent) > >> + * @see > >> + * > java.awt.event.ActionListener#actionPerformed(java.awt.event. > >> + * ActionEvent) > >> */ > >> public void actionPerformed(ActionEvent e) { > >> doBrowse(); > >> @@ -192,11 +195,10 @@ > >> if (!inModel) { > >> outputDirectoryComboBox.setEditable(true); > >> JPanel outputPanel = new JPanel(new BorderLayout(5, > 0)); > >> - outputPanel.setBorder( > >> - BorderFactory.createCompoundBorder( > >> - BorderFactory.createTitledBorder( > >> - > Translator.localize("label.output-directory")), > >> - BorderFactory.createEmptyBorder(2, 5, 5, > 5))); > >> + > outputPanel.setBorder(BorderFactory.createCompoundBorder( > >> + > BorderFactory.createTitledBorder(Translator > >> + > .localize("label.output-directory")), BorderFactory > >> + .createEmptyBorder(2, 5, 5, > 5))); > >> outputPanel.add(outputDirectoryComboBox, > BorderLayout.CENTER); > >> outputPanel.add(browseButton, BorderLayout.EAST); > >> southPanel.add(outputPanel, BorderLayout.NORTH); > >> @@ -204,18 +206,18 @@ > >> > >> // Compile Checkbox > >> > >> - //_compileCheckBox = new JCheckBox(); > >> - //nameButton(_compileCheckBox, > "checkbox.compile-generated-source"); > >> + // _compileCheckBox = new JCheckBox(); > >> + // nameButton(_compileCheckBox, > "checkbox.compile-generated-source"); > >> // TODO: Implement the compile feature. For now, disable > the checkbox. > >> - //_compileCheckBox.setEnabled(false); > >> - //southPanel.add(_compileCheckBox, BorderLayout.SOUTH); > >> + // _compileCheckBox.setEnabled(false); > >> + // southPanel.add(_compileCheckBox, BorderLayout.SOUTH); > >> > >> contentPanel.add(southPanel, BorderLayout.SOUTH); > >> > >> setContent(contentPanel); > >> > >> // TODO: Get saved default directory > >> -// > outputDirectoryComboBox.getModel().setSelectedItem(savedDir); > >> + // > outputDirectoryComboBox.getModel().setSelectedItem(savedDir); > >> } > >> > >> /* > >> @@ -238,14 +240,9 @@ > >> > >> JTableHeader header = classTable.getTableHeader(); > >> if (header != null) { > >> - c = > >> - > header.getDefaultRenderer().getTableCellRendererComponent( > >> - classTable, > >> - column.getHeaderValue(), > >> - false, > >> - false, > >> - 0, > >> - 0); > >> + c = header.getDefaultRenderer() > >> + > .getTableCellRendererComponent(classTable, > >> + > column.getHeaderValue(), false, false, 0, 0); > >> width = Math.max(c.getPreferredSize().width + > 8, width); > >> } > >> > >> @@ -257,8 +254,24 @@ > >> } > >> > >> private void buildLanguages() { > >> - languages = new ArrayList<Language>( > >> - > GeneratorManager.getInstance().getLanguages()); > >> + ArrayList<Language> tmp = new > ArrayList<Language>(GeneratorManager > >> + .getInstance().getLanguages()); > >> + > >> + languages = new ArrayList<Language>(); > >> + > >> + for (int i = 0, length = tmp.size(); i < length; i++) { > >> + Language current = tmp.get(i); > >> + int index = languages.size(); > >> + > >> + for (int j = 0, length_j = languages.size(); j < > length_j; j++) { > >> + if (current.getName().compareToIgnoreCase( > >> + languages.get(j).getName()) < 0) { > >> + index = j; > >> + break; > >> + } > >> + } > >> + languages.add(index, current); > >> + } > >> } > >> > >> private static Collection<String> getClasspathEntries() { > >> @@ -266,15 +279,15 @@ > >> Collection<String> entries = new TreeSet<String>(); > >> > >> // TODO: What does the output directory have to do with the > class path? > >> -// Project p = > ProjectManager.getManager().getCurrentProject(); > >> -// > entries.add(p.getProjectSettings().getGenerationOutputDir()); > >> + // Project p = > ProjectManager.getManager().getCurrentProject(); > >> + // > entries.add(p.getProjectSettings().getGenerationOutputDir()); > >> > >> final String pathSep = > System.getProperty("path.separator"); > >> StringTokenizer allEntries = new StringTokenizer(classpath, > pathSep); > >> while (allEntries.hasMoreElements()) { > >> String entry = allEntries.nextToken(); > >> if (!entry.toLowerCase().endsWith(".jar") > >> - && !entry.toLowerCase().endsWith(".zip")) { > >> + && > !entry.toLowerCase().endsWith(".zip")) { > >> entries.add(entry); > >> } > >> } > >> @@ -282,7 +295,8 @@ > >> } > >> > >> /* > >> - * @see > java.awt.event.ActionListener#actionPerformed(java.awt.event.ActionEvent) > >> + * @see > >> + * > java.awt.event.ActionListener#actionPerformed(java.awt.event.ActionEvent) > >> */ > >> @Override > >> public void actionPerformed(ActionEvent e) { > >> @@ -303,47 +317,43 @@ > >> Set nodes = > classTableModel.getChecked(language); > >> > >> if (!isPathInModel) { > >> - path = > >> - ((String) > outputDirectoryComboBox.getModel() > >> - .getSelectedItem()); > >> + path = ((String) > outputDirectoryComboBox.getModel() > >> + .getSelectedItem()); > >> if (path != null) { > >> path = path.trim(); > >> if (path.length() > 0) { > >> - Collection<String> files = > >> - > generator.generateFiles(nodes, path, false); > >> + Collection<String> files = > generator.generateFiles( > >> + nodes, path, > false); > >> for (String filename : files) > { > >> - fileNames[i].add( > >> - path + > CodeGenerator.FILE_SEPARATOR > >> + fileNames[i].add(path > >> + + > CodeGenerator.FILE_SEPARATOR > >> + > filename); > >> } > >> } > >> } > >> } else { > >> // classify nodes by base path > >> - Map<String, Set<Object>> nodesPerPath = > >> - new HashMap<String, > Set<Object>>(); > >> + Map<String, Set<Object>> nodesPerPath = > new HashMap<String, Set<Object>>(); > >> for (Object node : nodes) { > >> if > (!Model.getFacade().isAClassifier(node)) { > >> continue; > >> } > >> path = > GeneratorManager.getCodePath(node); > >> if (path == null) { > >> - Object parent = > >> - > Model.getFacade().getNamespace(node); > >> + Object parent = > Model.getFacade() > >> + > .getNamespace(node); > >> while (parent != null) { > >> path = > GeneratorManager.getCodePath(parent); > >> if (path != null) { > >> break; > >> } > >> - parent = > >> - > Model.getFacade().getNamespace(parent); > >> + parent = > Model.getFacade().getNamespace(parent); > >> } > >> } > >> if (path != null) { > >> final String fileSep = > CodeGenerator.FILE_SEPARATOR; > >> if (path.endsWith(fileSep)) { > // remove trailing / > >> - path = > >> - > path.substring(0, path.length() > >> + path = > path.substring(0, path.length() > >> - > fileSep.length()); > >> } > >> Set<Object> np = > nodesPerPath.get(path); > >> @@ -362,12 +372,11 @@ > >> Set nodeColl = (Set) > entry.getValue(); > >> // TODO: the last argument > (recursive flag) should be a > >> // selectable option > >> - Collection<String> files = > >> - > generator.generateFiles(nodeColl, basepath, false); > >> + Collection<String> files = > generator.generateFiles( > >> + nodeColl, basepath, > false); > >> for (String filename : files) { > >> fileNames[i].add(basepath > >> - + > CodeGenerator.FILE_SEPARATOR > >> - + filename); > >> + + > CodeGenerator.FILE_SEPARATOR + filename); > >> } > >> } > >> } // end if (!isPathInModel) .. else > >> @@ -380,26 +389,25 @@ > >> /** > >> * Save the source language in the model. > >> * > >> - * TODO: Support multiple languages now that we have UML 1.4 > >> - * tagged values. > >> + * TODO: Support multiple languages now that we have UML 1.4 > tagged values. > >> + * > >> * @param node > >> * @param language > >> */ > >> private void saveLanguage(Object node, Language language) { > >> - Object taggedValue = > >> - Model.getFacade().getTaggedValue(node, > SOURCE_LANGUAGE_TAG); > >> + Object taggedValue = > Model.getFacade().getTaggedValue(node, > >> + SOURCE_LANGUAGE_TAG); > >> if (taggedValue != null) { > >> String savedLang = > Model.getFacade().getValueOfTag(taggedValue); > >> if (!language.getName().equals(savedLang)) { > >> - > Model.getExtensionMechanismsHelper().setValueOfTag( > >> - taggedValue, language.getName()); > >> + > Model.getExtensionMechanismsHelper().setValueOfTag(taggedValue, > >> + language.getName()); > >> } > >> } else { > >> - taggedValue = > >> - > Model.getExtensionMechanismsFactory().buildTaggedValue( > >> - SOURCE_LANGUAGE_TAG, > language.getName()); > >> - Model.getExtensionMechanismsHelper().addTaggedValue( > >> - node, taggedValue); > >> + taggedValue = Model.getExtensionMechanismsFactory() > >> + .buildTaggedValue(SOURCE_LANGUAGE_TAG, > language.getName()); > >> + > Model.getExtensionMechanismsHelper().addTaggedValue(node, > >> + taggedValue); > >> > >> } > >> } > >> @@ -407,11 +415,9 @@ > >> private void doBrowse() { > >> try { > >> // Show Filechooser to select OutputDirectory > >> - JFileChooser chooser = > >> - new JFileChooser( > >> - (String) outputDirectoryComboBox > >> - .getModel() > >> - .getSelectedItem()); > >> + JFileChooser chooser = new JFileChooser( > >> + (String) > outputDirectoryComboBox.getModel() > >> + .getSelectedItem()); > >> > >> if (chooser == null) { > >> chooser = new JFileChooser(); > >> @@ -420,10 +426,10 @@ > >> chooser.setFileHidingEnabled(true); > >> chooser.setMultiSelectionEnabled(false); > >> > chooser.setFileSelectionMode(JFileChooser.DIRECTORIES_ONLY); > >> - chooser.setDialogTitle(Translator.localize( > >> - > "dialog.generation.chooser.choose-output-dir")); > >> - chooser.showDialog(this, Translator.localize( > >> - > "dialog.generation.chooser.approve-button-text")); > >> + chooser.setDialogTitle(Translator > >> + > .localize("dialog.generation.chooser.choose-output-dir")); > >> + chooser.showDialog(this, Translator > >> + > .localize("dialog.generation.chooser.approve-button-text")); > >> > >> if (!"".equals(chooser.getSelectedFile().getPath())) > { > >> String path = > chooser.getSelectedFile().getPath(); > >> @@ -446,9 +452,9 @@ > >> private List<Object> classes; > >> > >> /** > >> - * Array of sets of UML elements that the user has > selected. One set > >> - * per language. The Object typed objects are actually > UML Elements, > >> - * but we don't have a visible type for that. > >> + * Array of sets of UML elements that the user has > selected. One set per > >> + * language. The Object typed objects are actually UML > Elements, but we > >> + * don't have a visible type for that. > >> */ > >> private Set<Object>[] checked; > >> > >> @@ -458,7 +464,6 @@ > >> public TableModelClassChecks() { > >> } > >> > >> - > >> /** > >> * Set the target. > >> * > >> @@ -474,8 +479,7 @@ > >> > >> for (Object cls : classes) { > >> for (int j = 0; j < getLanguagesCount(); j++) { > >> - if (isSupposedToBeGeneratedAsLanguage( > >> - languages.get(j), cls)) { > >> + if > (isSupposedToBeGeneratedAsLanguage(languages.get(j), cls)) { > >> checked[j].add(cls); > >> } else if > ((languages.get(j)).getName().equals( > >> > Notation.getConfiguredNotation() > >> @@ -486,19 +490,18 @@ > >> } > >> fireTableStructureChanged(); > >> > >> - getOkButton().setEnabled(classes.size() > 0 > >> - && > getChecked().size() > 0); > >> + getOkButton().setEnabled( > >> + classes.size() > 0 && > getChecked().size() > 0); > >> } > >> > >> - private boolean isSupposedToBeGeneratedAsLanguage( > >> - Language lang, > >> - Object cls) { > >> + private boolean isSupposedToBeGeneratedAsLanguage(Language > lang, > >> + Object cls) { > >> if (lang == null || cls == null) { > >> return false; > >> } > >> > >> - Object taggedValue = > >> - Model.getFacade().getTaggedValue(cls, > SOURCE_LANGUAGE_TAG); > >> + Object taggedValue = > Model.getFacade().getTaggedValue(cls, > >> + SOURCE_LANGUAGE_TAG); > >> if (taggedValue == null) { > >> return false; > >> } > >> @@ -515,6 +518,7 @@ > >> > >> /** > >> * Return the set of elements which are selected for the > given language. > >> + * > >> * @param lang the language > >> * @return a set of UML elements > >> */ > >> @@ -528,7 +532,7 @@ > >> > >> /** > >> * All checked classes. > >> - * > >> + * > >> * @return The union of all languages as a {...@link Set}. > >> */ > >> public Set<Object> getChecked() { > >> @@ -539,9 +543,9 @@ > >> return union; > >> } > >> > >> - //////////////// > >> + // ////////////// > >> // TableModel implementation > >> - > >> + > >> /* > >> * @see javax.swing.table.TableModel#getColumnCount() > >> */ > >> @@ -624,20 +628,20 @@ > >> } > >> > >> /* > >> - * @see javax.swing.table.TableModel#setValueAt( > >> - * java.lang.Object, int, int) > >> + * @see javax.swing.table.TableModel#setValueAt( > java.lang.Object, int, > >> + * int) > >> */ > >> @Override > >> public void setValueAt(Object aValue, int rowIndex, int > columnIndex) { > >> if (columnIndex == getLanguagesCount()) { > >> return; > >> - } > >> + } > >> if (columnIndex >= getColumnCount()) { > >> return; > >> - } > >> + } > >> if (!(aValue instanceof Boolean)) { > >> return; > >> - } > >> + } > >> boolean val = ((Boolean) aValue).booleanValue(); > >> Object cls = classes.get(rowIndex); > >> > >> @@ -652,19 +656,18 @@ > >> if (val && !getOkButton().isEnabled()) { > >> getOkButton().setEnabled(true); > >> } else if (!val && getOkButton().isEnabled() > >> - && getChecked().size() == 0) { > >> + && getChecked().size() == 0) { > >> getOkButton().setEnabled(false); > >> } > >> } > >> > >> /** > >> - * Sets or clears all checkmarks for the (next) language > for > >> - * all classes. > >> - * > >> + * Sets or clears all checkmarks for the (next) language > for all > >> + * classes. > >> + * > >> * @param value If false then all checkmarks are cleared > for all > >> - * languages. > >> - * If true then all are cleared, except for one language > column, > >> - * these are all set. > >> + * languages. If true then all are > cleared, except for one > >> + * language column, these are all set. > >> */ > >> public void setAllChecks(boolean value) { > >> int rows = getRowCount(); > >> @@ -686,10 +689,10 @@ > >> } > >> } > >> if (value) { > >> - if (++languageHistory >= checks) { > >> - languageHistory = 0; > >> - } > >> - } > >> + if (++languageHistory >= checks) { > >> + languageHistory = 0; > >> + } > >> + } > >> getOkButton().setEnabled(value); > >> } > >> > >> @@ -704,4 +707,3 @@ > >> */ > >> private static final long serialVersionUID = > -8897965616334156746L; > >> } /* end class ClassGenerationDialog */ > >> - > >> > >> ------------------------------------------------------ > >> > http://argouml.tigris.org/ds/viewMessage.do?dsForumId=5905&dsMessageId=1404840 > >> > >> To unsubscribe from this discussion, e-mail: > [[email protected]]. > >> > > > > > > > > -- > > Cheers, > > > > Christian López Espínola <penyaskito> > > > > ------------------------------------------------------ > > > http://argouml.tigris.org/ds/viewMessage.do?dsForumId=450&dsMessageId=1406817 > > > > To unsubscribe from this discussion, e-mail: > [[email protected]]. > > To be allowed to post to the list contact the mailing list moderator, > email: [[email protected]] > > ------------------------------------------------------ > http://argouml.tigris.org/ds/viewMessage.do?dsForumId=450&dsMessageId=1408132 > > To unsubscribe from this discussion, e-mail: > [[email protected]]. > To be allowed to post to the list contact the mailing list moderator, > email: [[email protected]] -- Psssst! Schon vom neuen GMX MultiMessenger gehört? Der kann`s mit allen: http://www.gmx.net/de/go/multimessenger01 ------------------------------------------------------ http://argouml.tigris.org/ds/viewMessage.do?dsForumId=450&dsMessageId=1412026 To unsubscribe from this discussion, e-mail: [[email protected]]. To be allowed to post to the list contact the mailing list moderator, email: [[email protected]]
