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]]
