From 3c6db128004b53233488885c604f1b49428e93b5 Mon Sep 17 00:00:00 2001 From: JonasSchaub Date: Wed, 26 Apr 2023 17:01:21 +0200 Subject: [PATCH 1/4] Renamed ListUtil to CollectionUtil, wip; --- .../controller/HistogramViewController.java | 6 +++--- .../cheminf/mortar/gui/util/GuiUtil.java | 4 ++-- .../ErtlFunctionalGroupsFinderFragmenter.java | 2 +- .../util/{ListUtil.java => CollectionUtil.java} | 17 ++++++++++++----- 4 files changed, 18 insertions(+), 11 deletions(-) rename src/main/java/de/unijena/cheminf/mortar/model/util/{ListUtil.java => CollectionUtil.java} (94%) diff --git a/src/main/java/de/unijena/cheminf/mortar/controller/HistogramViewController.java b/src/main/java/de/unijena/cheminf/mortar/controller/HistogramViewController.java index 977ae1d6..b0307d37 100644 --- a/src/main/java/de/unijena/cheminf/mortar/controller/HistogramViewController.java +++ b/src/main/java/de/unijena/cheminf/mortar/controller/HistogramViewController.java @@ -27,7 +27,7 @@ import de.unijena.cheminf.mortar.model.data.FragmentDataModel; import de.unijena.cheminf.mortar.model.data.MoleculeDataModel; import de.unijena.cheminf.mortar.model.depict.DepictionUtil; -import de.unijena.cheminf.mortar.model.util.ListUtil; +import de.unijena.cheminf.mortar.model.util.CollectionUtil; import javafx.beans.binding.Bindings; import javafx.beans.value.ObservableValue; @@ -269,7 +269,7 @@ private BarChart createHistogram(int aFragmentNumber, HistogramView aHistogramVi ArrayList tmpFullSmilesLength = new ArrayList<>(); String tmpSelectedData = (String) aHistogramView.getChooseDataComoBox().getValue(); if (tmpSelectedData.equals(Message.get("HistogramView.chooseDataComboBoxFragmentFrequency.text"))) { - ListUtil.sortGivenFragmentListByPropertyAndSortType(this.copyList, "absoluteFrequency", "ASCENDING"); + CollectionUtil.sortGivenFragmentListByPropertyAndSortType(this.copyList, "absoluteFrequency", "ASCENDING"); for (MoleculeDataModel tmpMoleculeData : this.copyList) { tmpFragmentData = (FragmentDataModel) tmpMoleculeData; if (tmpFragmentData.getUniqueSmiles().length() > aSmilesLength) { @@ -284,7 +284,7 @@ private BarChart createHistogram(int aFragmentNumber, HistogramView aHistogramVi tmpFrequencyList.add(tmpFragmentData.getAbsoluteFrequency()); } } else { - ListUtil.sortGivenFragmentListByPropertyAndSortType(this.copyList, "moleculeFrequency", "ASCENDING"); + CollectionUtil.sortGivenFragmentListByPropertyAndSortType(this.copyList, "moleculeFrequency", "ASCENDING"); for (MoleculeDataModel tmpMoleculeData : this.copyList) { tmpFragmentData = (FragmentDataModel) tmpMoleculeData; if (tmpFragmentData.getUniqueSmiles().length() > aSmilesLength) { diff --git a/src/main/java/de/unijena/cheminf/mortar/gui/util/GuiUtil.java b/src/main/java/de/unijena/cheminf/mortar/gui/util/GuiUtil.java index 03af91b7..de0c6298 100644 --- a/src/main/java/de/unijena/cheminf/mortar/gui/util/GuiUtil.java +++ b/src/main/java/de/unijena/cheminf/mortar/gui/util/GuiUtil.java @@ -28,7 +28,7 @@ import de.unijena.cheminf.mortar.model.data.MoleculeDataModel; import de.unijena.cheminf.mortar.model.depict.DepictionUtil; import de.unijena.cheminf.mortar.model.settings.SettingsContainer; -import de.unijena.cheminf.mortar.model.util.ListUtil; +import de.unijena.cheminf.mortar.model.util.CollectionUtil; import javafx.scene.control.Alert; import javafx.scene.control.Button; import javafx.scene.control.ButtonType; @@ -212,7 +212,7 @@ public static void sortTableViewGlobally(SortEvent anEvent, Paginatio return; String tmpSortProp = ((PropertyValueFactory)((TableColumn) anEvent.getSource().getSortOrder().get(0)).cellValueFactoryProperty().getValue()).getProperty().toString(); TableColumn.SortType tmpSortType = ((TableColumn) anEvent.getSource().getSortOrder().get(0)).getSortType(); - ListUtil.sortGivenFragmentListByPropertyAndSortType(((IDataTableView)anEvent.getSource()).getItemsList(), tmpSortProp, tmpSortType.toString()); + CollectionUtil.sortGivenFragmentListByPropertyAndSortType(((IDataTableView)anEvent.getSource()).getItemsList(), tmpSortProp, tmpSortType.toString()); int fromIndex = tmpPagination.getCurrentPageIndex() * tmpRowsPerPage; int toIndex = Math.min(fromIndex + tmpRowsPerPage, ((IDataTableView)anEvent.getSource()).getItemsList().size()); anEvent.getSource().getItems().clear(); diff --git a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ErtlFunctionalGroupsFinderFragmenter.java b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ErtlFunctionalGroupsFinderFragmenter.java index c788e4e3..4386c677 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ErtlFunctionalGroupsFinderFragmenter.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ErtlFunctionalGroupsFinderFragmenter.java @@ -338,7 +338,7 @@ public static enum CycleFinderOption { * Constructor, all settings are initialised with their default values as declared in the respective public constants. */ public ErtlFunctionalGroupsFinderFragmenter() { - this.settingNameTooltipTextMap = new HashMap(10, 0.9f); + this.settingNameTooltipTextMap = new HashMap(6 * (4/3) + 1, 0.75f); this.fragmentSaturationSetting = new SimpleEnumConstantNameProperty(this, "Fragment saturation setting", IMoleculeFragmenter.FRAGMENT_SATURATION_OPTION_DEFAULT.name(), IMoleculeFragmenter.FragmentSaturationOption.class) { @Override diff --git a/src/main/java/de/unijena/cheminf/mortar/model/util/ListUtil.java b/src/main/java/de/unijena/cheminf/mortar/model/util/CollectionUtil.java similarity index 94% rename from src/main/java/de/unijena/cheminf/mortar/model/util/ListUtil.java rename to src/main/java/de/unijena/cheminf/mortar/model/util/CollectionUtil.java index 32dd8d95..14999491 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/util/ListUtil.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/util/CollectionUtil.java @@ -27,15 +27,15 @@ import java.util.List; /** - * Util class for lists. + * Util class for collections. * - * @author Felix Baensch - * @version 1.0.0.0 + * @author Felix Baensch, Jonas Schaub + * @version 1.0.1.0 */ -public final class ListUtil { +public final class CollectionUtil { // /** - * Sorts given list by property and sort type + * Sorts given list by property and sort type. * * @param aList List * @param aProperty String @@ -109,5 +109,12 @@ public static void sortGivenFragmentListByPropertyAndSortType(List } From 68c16cb7da97d6d92f728028d981ad2b35ffa2c3 Mon Sep 17 00:00:00 2001 From: JonasSchaub Date: Thu, 27 Apr 2023 17:20:45 +0200 Subject: [PATCH 2/4] Implemented routine to calculate a suitable initial HashMap size based on the number of elements to be stored and load factor; implemented its usage in all places in the MORTAR-own classes where new HashMaps are instantiated; --- .../FragmentationSettingsViewController.java | 5 +- .../mortar/controller/MainViewController.java | 3 +- .../controller/SettingsViewController.java | 3 +- .../fragmentation/FragmentationTask.java | 3 +- .../ErtlFunctionalGroupsFinderFragmenter.java | 7 ++- .../ScaffoldGeneratorFragmenter.java | 4 +- .../SugarRemovalUtilityFragmenter.java | 4 +- .../model/settings/SettingsContainer.java | 4 +- .../mortar/model/util/CollectionUtil.java | 39 +++++++++++++- .../mortar/model/util/CollectionUtilTest.java | 51 +++++++++++++++++++ 10 files changed, 111 insertions(+), 12 deletions(-) create mode 100644 src/test/java/de/unijena/cheminf/mortar/model/util/CollectionUtilTest.java diff --git a/src/main/java/de/unijena/cheminf/mortar/controller/FragmentationSettingsViewController.java b/src/main/java/de/unijena/cheminf/mortar/controller/FragmentationSettingsViewController.java index cc72f1f1..2ce60b4b 100644 --- a/src/main/java/de/unijena/cheminf/mortar/controller/FragmentationSettingsViewController.java +++ b/src/main/java/de/unijena/cheminf/mortar/controller/FragmentationSettingsViewController.java @@ -24,6 +24,7 @@ import de.unijena.cheminf.mortar.gui.views.SettingsView; import de.unijena.cheminf.mortar.message.Message; import de.unijena.cheminf.mortar.model.fragmentation.algorithm.IMoleculeFragmenter; +import de.unijena.cheminf.mortar.model.util.CollectionUtil; import javafx.beans.property.Property; import javafx.scene.Scene; import javafx.scene.control.Tab; @@ -85,7 +86,7 @@ public class FragmentationSettingsViewController { */ public FragmentationSettingsViewController(Stage aStage, IMoleculeFragmenter[] anArrayOfFragmenters, String aSelectedFragmenterAlgorithmName){ this.mainStage = aStage; - this.recentProperties = new HashMap<>(anArrayOfFragmenters.length); + this.recentProperties = new HashMap<>(CollectionUtil.calculateInitialHashMapCapacity(anArrayOfFragmenters.length)); this.fragmenters = anArrayOfFragmenters; this.selectedFragmenterName = aSelectedFragmenterAlgorithmName; this.openFragmentationSettingsView(); @@ -111,7 +112,7 @@ private void openFragmentationSettingsView(){ // this.addListener(); for (IMoleculeFragmenter tmpFragmenter : this.fragmenters) { - HashMap tmpRecentProperties = new HashMap<>(tmpFragmenter.settingsProperties().size()); + HashMap tmpRecentProperties = new HashMap<>(CollectionUtil.calculateInitialHashMapCapacity(tmpFragmenter.settingsProperties().size())); this.recentProperties.put(tmpFragmenter.getFragmentationAlgorithmName(), tmpRecentProperties); Tab tmpTab = this.settingsView.addTab(this.fragmentationSettingsViewStage, tmpFragmenter.getFragmentationAlgorithmName(), tmpFragmenter.settingsProperties(), diff --git a/src/main/java/de/unijena/cheminf/mortar/controller/MainViewController.java b/src/main/java/de/unijena/cheminf/mortar/controller/MainViewController.java index d5d888a2..1eb6ca0a 100644 --- a/src/main/java/de/unijena/cheminf/mortar/controller/MainViewController.java +++ b/src/main/java/de/unijena/cheminf/mortar/controller/MainViewController.java @@ -40,6 +40,7 @@ import de.unijena.cheminf.mortar.model.settings.SettingsContainer; import de.unijena.cheminf.mortar.model.util.BasicDefinitions; import de.unijena.cheminf.mortar.model.util.ChemUtil; +import de.unijena.cheminf.mortar.model.util.CollectionUtil; import de.unijena.cheminf.mortar.model.util.FileUtil; import de.unijena.cheminf.mortar.model.util.LogUtil; import javafx.application.Platform; @@ -239,7 +240,7 @@ public MainViewController(Stage aStage, MainView aMainView, String anAppDir) { // this.isImportRunningProperty = new SimpleBooleanProperty(false); this.isExportRunningProperty = new SimpleBooleanProperty(false); - this.mapOfFragmentDataModelLists = new HashMap<>(5); + this.mapOfFragmentDataModelLists = new HashMap<>(CollectionUtil.calculateInitialHashMapCapacity(5)); this.threadList = new CopyOnWriteArrayList(); this.addListener(); this.addFragmentationAlgorithmCheckMenuItems(); diff --git a/src/main/java/de/unijena/cheminf/mortar/controller/SettingsViewController.java b/src/main/java/de/unijena/cheminf/mortar/controller/SettingsViewController.java index ebeb9476..07324652 100644 --- a/src/main/java/de/unijena/cheminf/mortar/controller/SettingsViewController.java +++ b/src/main/java/de/unijena/cheminf/mortar/controller/SettingsViewController.java @@ -24,6 +24,7 @@ import de.unijena.cheminf.mortar.gui.views.SettingsView; import de.unijena.cheminf.mortar.message.Message; import de.unijena.cheminf.mortar.model.settings.SettingsContainer; +import de.unijena.cheminf.mortar.model.util.CollectionUtil; import javafx.application.Platform; import javafx.beans.property.Property; import javafx.scene.Scene; @@ -86,7 +87,7 @@ public SettingsViewController(Stage aStage, SettingsContainer aSettingsContainer this.mainStage = aStage; this.settingsContainer = aSettingsContainer; this.recentSettingsContainer = aSettingsContainer; - this.recentProperties = new HashMap<>(this.settingsContainer.settingsProperties().size()); + this.recentProperties = new HashMap<>(CollectionUtil.calculateInitialHashMapCapacity(this.settingsContainer.settingsProperties().size())); this.showSettingsView(); } // diff --git a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/FragmentationTask.java b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/FragmentationTask.java index cfefc5c2..a9ad6e90 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/FragmentationTask.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/FragmentationTask.java @@ -24,6 +24,7 @@ import de.unijena.cheminf.mortar.model.data.MoleculeDataModel; import de.unijena.cheminf.mortar.model.fragmentation.algorithm.IMoleculeFragmenter; import de.unijena.cheminf.mortar.model.util.ChemUtil; +import de.unijena.cheminf.mortar.model.util.CollectionUtil; import org.openscience.cdk.exception.CDKException; import org.openscience.cdk.interfaces.IAtomContainer; @@ -134,7 +135,7 @@ public Integer call() throws Exception{ continue; } List tmpFragmentsOfMolList = new ArrayList<>(tmpFragmentsList.size()); - HashMap tmpFragmentFrequenciesOfMoleculeMap = new HashMap<>(tmpFragmentsList.size()); + HashMap tmpFragmentFrequenciesOfMoleculeMap = new HashMap<>(CollectionUtil.calculateInitialHashMapCapacity(tmpFragmentsList.size())); for(IAtomContainer tmpFragment : tmpFragmentsList){ String tmpSmiles = ChemUtil.createUniqueSmiles(tmpFragment); if (tmpSmiles == null) { diff --git a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ErtlFunctionalGroupsFinderFragmenter.java b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ErtlFunctionalGroupsFinderFragmenter.java index 4386c677..bbc0a4b8 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ErtlFunctionalGroupsFinderFragmenter.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ErtlFunctionalGroupsFinderFragmenter.java @@ -29,6 +29,7 @@ import de.unijena.cheminf.mortar.message.Message; import de.unijena.cheminf.mortar.model.io.Importer; import de.unijena.cheminf.mortar.model.util.ChemUtil; +import de.unijena.cheminf.mortar.model.util.CollectionUtil; import de.unijena.cheminf.mortar.model.util.SimpleEnumConstantNameProperty; import javafx.beans.property.Property; import javafx.beans.property.SimpleBooleanProperty; @@ -338,7 +339,8 @@ public static enum CycleFinderOption { * Constructor, all settings are initialised with their default values as declared in the respective public constants. */ public ErtlFunctionalGroupsFinderFragmenter() { - this.settingNameTooltipTextMap = new HashMap(6 * (4/3) + 1, 0.75f); + int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashMapCapacity(6, 0.75f); + this.settingNameTooltipTextMap = new HashMap(tmpInitialCapacityForSettingNameTooltipTextMap, 0.75f); this.fragmentSaturationSetting = new SimpleEnumConstantNameProperty(this, "Fragment saturation setting", IMoleculeFragmenter.FRAGMENT_SATURATION_OPTION_DEFAULT.name(), IMoleculeFragmenter.FragmentSaturationOption.class) { @Override @@ -805,7 +807,8 @@ public List fragmentMolecule(IAtomContainer aMolecule) this.logger.log(Level.WARNING, anException.toString(), anException); throw new IllegalArgumentException("Unexpected error at aromaticity detection: " + anException.toString()); } - HashMap tmpIdToAtomMap = new HashMap<>(tmpMoleculeClone.getAtomCount() + 1, 1); + int tmpInitialCapacityForIdToAtomMap = CollectionUtil.calculateInitialHashMapCapacity(tmpMoleculeClone.getAtomCount(), 0.75f); + HashMap tmpIdToAtomMap = new HashMap<>(tmpInitialCapacityForIdToAtomMap, 0.75f); for (int i = 0; i < tmpMoleculeClone.getAtomCount(); i++) { IAtom tmpAtom = tmpMoleculeClone.getAtom(i); tmpAtom.setProperty(ErtlFunctionalGroupsFinderFragmenter.INTERNAL_INDEX_PROPERTY_KEY, i); diff --git a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ScaffoldGeneratorFragmenter.java b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ScaffoldGeneratorFragmenter.java index 7c27b07a..7fe3b5bf 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ScaffoldGeneratorFragmenter.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ScaffoldGeneratorFragmenter.java @@ -22,6 +22,7 @@ import de.unijena.cheminf.mortar.gui.util.GuiUtil; import de.unijena.cheminf.mortar.message.Message; +import de.unijena.cheminf.mortar.model.util.CollectionUtil; import de.unijena.cheminf.mortar.model.util.SimpleEnumConstantNameProperty; import de.unijena.cheminf.scaffoldGenerator.ScaffoldGenerator; import javafx.beans.property.Property; @@ -340,7 +341,8 @@ public static enum FragmentationTypeOption { */ public ScaffoldGeneratorFragmenter() { this.scaffoldGeneratorInstance = new ScaffoldGenerator(); - this.settingNameTooltipTextMap = new HashMap(16, 0.9f); + int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashMapCapacity(16, 0.75f); + this.settingNameTooltipTextMap = new HashMap(tmpInitialCapacityForSettingNameTooltipTextMap, 0.75f); this.fragmentSaturationSetting = new SimpleEnumConstantNameProperty(this, "Fragment saturation setting", IMoleculeFragmenter.FRAGMENT_SATURATION_OPTION_DEFAULT.name(), IMoleculeFragmenter.FragmentSaturationOption.class) { @Override diff --git a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/SugarRemovalUtilityFragmenter.java b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/SugarRemovalUtilityFragmenter.java index 98f040dd..79704b92 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/SugarRemovalUtilityFragmenter.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/SugarRemovalUtilityFragmenter.java @@ -29,6 +29,7 @@ import de.unijena.cheminf.mortar.gui.util.GuiUtil; import de.unijena.cheminf.mortar.message.Message; import de.unijena.cheminf.mortar.model.util.ChemUtil; +import de.unijena.cheminf.mortar.model.util.CollectionUtil; import de.unijena.cheminf.mortar.model.util.SimpleEnumConstantNameProperty; import javafx.beans.property.Property; import javafx.beans.property.SimpleBooleanProperty; @@ -235,7 +236,8 @@ public static enum SRUFragmenterReturnedFragmentsOption { public SugarRemovalUtilityFragmenter() { this.sugarRUInstance = new SugarRemovalUtility(SilentChemObjectBuilder.getInstance()); this.settings = new ArrayList<>(15); - this.settingNameTooltipTextMap = new HashMap<>(20, 0.9f); + int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashMapCapacity(20, 0.75f); + this.settingNameTooltipTextMap = new HashMap<>(tmpInitialCapacityForSettingNameTooltipTextMap, 0.75f); this.returnedFragmentsSetting = new SimpleEnumConstantNameProperty(this, "Returned fragments setting", SugarRemovalUtilityFragmenter.RETURNED_FRAGMENTS_OPTION_DEFAULT.name(), SugarRemovalUtilityFragmenter.SRUFragmenterReturnedFragmentsOption.class) { @Override diff --git a/src/main/java/de/unijena/cheminf/mortar/model/settings/SettingsContainer.java b/src/main/java/de/unijena/cheminf/mortar/model/settings/SettingsContainer.java index edb17b31..18eb30af 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/settings/SettingsContainer.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/settings/SettingsContainer.java @@ -28,6 +28,7 @@ import de.unijena.cheminf.mortar.gui.util.GuiUtil; import de.unijena.cheminf.mortar.message.Message; import de.unijena.cheminf.mortar.model.util.BasicDefinitions; +import de.unijena.cheminf.mortar.model.util.CollectionUtil; import de.unijena.cheminf.mortar.model.util.FileUtil; import de.unijena.cheminf.mortar.model.util.SimpleEnumConstantNameProperty; import de.unijena.cheminf.mortar.preference.BooleanPreference; @@ -609,7 +610,8 @@ public void reloadGlobalSettings() { * to the list of settings for display to the user. */ private void initialiseSettings() { - this.settingNameTooltipTextMap = new HashMap(10, 0.9f); + int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashMapCapacity(10, 0.75f); + this.settingNameTooltipTextMap = new HashMap(tmpInitialCapacityForSettingNameTooltipTextMap, 0.75f); this.rowsPerPageSetting = new SimpleIntegerProperty(this, "Rows per page setting", SettingsContainer.ROWS_PER_PAGE_SETTING_DEFAULT) { diff --git a/src/main/java/de/unijena/cheminf/mortar/model/util/CollectionUtil.java b/src/main/java/de/unijena/cheminf/mortar/model/util/CollectionUtil.java index 14999491..fd76826a 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/util/CollectionUtil.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/util/CollectionUtil.java @@ -111,10 +111,45 @@ public static void sortGivenFragmentListByPropertyAndSortType(ListThe initial capacity multiplied with the load factor (= rehash threshold) must be higher than the number of + * elements. * + * @param aNumberOfElements number of elements supposed to be stored in the new HashMap instance + * @param aLoadFactor load factor that is specified for the new HashMap instance + * @return a suitable initial size for the new HashMap instance that leads to a rehash threshold that is slightly + * higher than the number of elements + * @throws IllegalArgumentException if the number of elements or the load factor is negative or equal to zero + * or if the load factor is greater than 1.0 */ - public static int calculateInitialHashMapSize(int aSize, float aLoadingFacotr ) { - return 0; + public static int calculateInitialHashMapCapacity(int aNumberOfElements, float aLoadFactor) + throws IllegalArgumentException { + if (aNumberOfElements <= 0) { + throw new IllegalArgumentException("Number of elements needs to be higher than 0 but is " + aNumberOfElements); + } + if (aLoadFactor <= 0 || aLoadFactor > 1.0f) { + throw new IllegalArgumentException("Load factor must be higher than 0 and not bigger than 1.0 but is " + aLoadFactor); + } + float tmpInitialSize = (float) aNumberOfElements * (1.0f / aLoadFactor) + 2.0f; + return (int) tmpInitialSize; + } + // + /** + * Calculates a suitable initial size for instantiating a new HashMap instance based on the number of elements + * supposed to be stored in it and the default load factor (0.75) that determines the rehash threshold. + * Calculation: initCap = (int) aNumberOfElement * (1/0.75) + 2 + *
The initial capacity multiplied with the load factor (= rehash threshold) must be higher than the number of + * elements. + * + * @param aNumberOfElements number of elements supposed to be stored in the new HashMap instance + * @return a suitable initial size for the new HashMap instance that leads to a rehash threshold that is slightly + * higher than the number of elements + * @throws IllegalArgumentException if the number of elements is negative or equal to zero + */ + public static int calculateInitialHashMapCapacity(int aNumberOfElements) throws IllegalArgumentException { + return CollectionUtil.calculateInitialHashMapCapacity(aNumberOfElements, 0.75f); } // } diff --git a/src/test/java/de/unijena/cheminf/mortar/model/util/CollectionUtilTest.java b/src/test/java/de/unijena/cheminf/mortar/model/util/CollectionUtilTest.java new file mode 100644 index 00000000..610a3340 --- /dev/null +++ b/src/test/java/de/unijena/cheminf/mortar/model/util/CollectionUtilTest.java @@ -0,0 +1,51 @@ +/* + * MORTAR - MOlecule fRagmenTAtion fRamework + * Copyright (C) 2023 Felix Baensch, Jonas Schaub (felix.baensch@w-hs.de, jonas.schaub@uni-jena.de) + * + * Source code is available at + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package de.unijena.cheminf.mortar.model.util; + +import org.junit.Assert; +import org.junit.Test; + +/** + * Test class for CollectionUtil routines. + */ +public class CollectionUtilTest { + /** + * Tests whether a few examples of number of elements and load factor pairs generate suitable initial capacities that + * lead to rehash thresholds higher than the number of elements using the specific calculation method from CollectionUtil. + */ + @Test + public void calculateInitialHashMapSizeTest() { + int[] tmpNumberOfElements = new int[]{10, 100, 4353456, 30}; + float[] tmpLoadFactor = new float[]{0.75f, 0.75f, 0.6f, 0.75f}; + int[] tmpExpectedInitialCapacity = new int[]{15, 135, 7255762, 42}; + for (int i = 0; i < tmpNumberOfElements.length; i++) { + int tmpCalculatedInitialHashMapCapacity = CollectionUtil.calculateInitialHashMapCapacity(tmpNumberOfElements[i], tmpLoadFactor[i]); + float tmpRehashThreshold = tmpCalculatedInitialHashMapCapacity * tmpLoadFactor[i]; + System.out.println("\nNumber of elements: " + tmpNumberOfElements[i]); + System.out.println("Load factor: " + tmpLoadFactor[i]); + System.out.println("Expected initial capacity ((int) n_elements * (1/lf) + 2): " + tmpExpectedInitialCapacity[i]); + System.out.println("Calculated initial capacity: " + tmpCalculatedInitialHashMapCapacity); + System.out.println("Rehash threshold (lf * initCap, must be higher than number of elements): " + tmpRehashThreshold); + Assert.assertTrue(tmpNumberOfElements[i] < tmpRehashThreshold); + Assert.assertEquals(tmpExpectedInitialCapacity[i], tmpCalculatedInitialHashMapCapacity); + } + } +} \ No newline at end of file From 2ff5c4667096a9bafe1cccda88e6645b6220d94f Mon Sep 17 00:00:00 2001 From: JonasSchaub Date: Fri, 28 Apr 2023 15:54:55 +0200 Subject: [PATCH 3/4] Extended usage of the new method to HashSet initialisations; introduced basic definitions constant for default hash collection load factor; --- .../FragmentationSettingsViewController.java | 4 ++-- .../mortar/controller/MainViewController.java | 2 +- .../controller/SettingsViewController.java | 2 +- .../fragmentation/FragmentationService.java | 12 +++++++---- .../fragmentation/FragmentationTask.java | 2 +- .../ErtlFunctionalGroupsFinderFragmenter.java | 9 +++++---- .../ScaffoldGeneratorFragmenter.java | 5 +++-- .../SugarRemovalUtilityFragmenter.java | 5 +++-- .../model/settings/SettingsContainer.java | 11 +++++----- .../mortar/model/util/BasicDefinitions.java | 4 ++++ .../mortar/model/util/CollectionUtil.java | 20 +++++++++---------- .../mortar/model/util/CollectionUtilTest.java | 2 +- 12 files changed, 45 insertions(+), 33 deletions(-) diff --git a/src/main/java/de/unijena/cheminf/mortar/controller/FragmentationSettingsViewController.java b/src/main/java/de/unijena/cheminf/mortar/controller/FragmentationSettingsViewController.java index 2ce60b4b..b77975f5 100644 --- a/src/main/java/de/unijena/cheminf/mortar/controller/FragmentationSettingsViewController.java +++ b/src/main/java/de/unijena/cheminf/mortar/controller/FragmentationSettingsViewController.java @@ -86,7 +86,7 @@ public class FragmentationSettingsViewController { */ public FragmentationSettingsViewController(Stage aStage, IMoleculeFragmenter[] anArrayOfFragmenters, String aSelectedFragmenterAlgorithmName){ this.mainStage = aStage; - this.recentProperties = new HashMap<>(CollectionUtil.calculateInitialHashMapCapacity(anArrayOfFragmenters.length)); + this.recentProperties = new HashMap<>(CollectionUtil.calculateInitialHashCollectionCapacity(anArrayOfFragmenters.length)); this.fragmenters = anArrayOfFragmenters; this.selectedFragmenterName = aSelectedFragmenterAlgorithmName; this.openFragmentationSettingsView(); @@ -112,7 +112,7 @@ private void openFragmentationSettingsView(){ // this.addListener(); for (IMoleculeFragmenter tmpFragmenter : this.fragmenters) { - HashMap tmpRecentProperties = new HashMap<>(CollectionUtil.calculateInitialHashMapCapacity(tmpFragmenter.settingsProperties().size())); + HashMap tmpRecentProperties = new HashMap<>(CollectionUtil.calculateInitialHashCollectionCapacity(tmpFragmenter.settingsProperties().size())); this.recentProperties.put(tmpFragmenter.getFragmentationAlgorithmName(), tmpRecentProperties); Tab tmpTab = this.settingsView.addTab(this.fragmentationSettingsViewStage, tmpFragmenter.getFragmentationAlgorithmName(), tmpFragmenter.settingsProperties(), diff --git a/src/main/java/de/unijena/cheminf/mortar/controller/MainViewController.java b/src/main/java/de/unijena/cheminf/mortar/controller/MainViewController.java index 1eb6ca0a..c16dec3b 100644 --- a/src/main/java/de/unijena/cheminf/mortar/controller/MainViewController.java +++ b/src/main/java/de/unijena/cheminf/mortar/controller/MainViewController.java @@ -240,7 +240,7 @@ public MainViewController(Stage aStage, MainView aMainView, String anAppDir) { // this.isImportRunningProperty = new SimpleBooleanProperty(false); this.isExportRunningProperty = new SimpleBooleanProperty(false); - this.mapOfFragmentDataModelLists = new HashMap<>(CollectionUtil.calculateInitialHashMapCapacity(5)); + this.mapOfFragmentDataModelLists = new HashMap<>(CollectionUtil.calculateInitialHashCollectionCapacity(5)); this.threadList = new CopyOnWriteArrayList(); this.addListener(); this.addFragmentationAlgorithmCheckMenuItems(); diff --git a/src/main/java/de/unijena/cheminf/mortar/controller/SettingsViewController.java b/src/main/java/de/unijena/cheminf/mortar/controller/SettingsViewController.java index 07324652..42993976 100644 --- a/src/main/java/de/unijena/cheminf/mortar/controller/SettingsViewController.java +++ b/src/main/java/de/unijena/cheminf/mortar/controller/SettingsViewController.java @@ -87,7 +87,7 @@ public SettingsViewController(Stage aStage, SettingsContainer aSettingsContainer this.mainStage = aStage; this.settingsContainer = aSettingsContainer; this.recentSettingsContainer = aSettingsContainer; - this.recentProperties = new HashMap<>(CollectionUtil.calculateInitialHashMapCapacity(this.settingsContainer.settingsProperties().size())); + this.recentProperties = new HashMap<>(CollectionUtil.calculateInitialHashCollectionCapacity(this.settingsContainer.settingsProperties().size())); this.showSettingsView(); } // diff --git a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/FragmentationService.java b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/FragmentationService.java index 41742977..d39d8225 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/FragmentationService.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/FragmentationService.java @@ -31,6 +31,7 @@ import de.unijena.cheminf.mortar.model.settings.SettingsContainer; import de.unijena.cheminf.mortar.model.util.BasicDefinitions; import de.unijena.cheminf.mortar.model.util.ChemUtil; +import de.unijena.cheminf.mortar.model.util.CollectionUtil; import de.unijena.cheminf.mortar.model.util.FileUtil; import de.unijena.cheminf.mortar.model.util.SimpleEnumConstantNameProperty; import de.unijena.cheminf.mortar.preference.BooleanPreference; @@ -1063,23 +1064,26 @@ private void updatePropertiesFromPreferences(List aPropertiesList, Pre * anything does not meet the requirements. */ private void checkFragmenters() throws Exception { - HashSet tmpAlgorithmNames = new HashSet<>(this.fragmenters.length + 6, 1.0f); + int tmpAlgorithmNamesSetInitCapacity = CollectionUtil.calculateInitialHashCollectionCapacity(this.fragmenters.length, + BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); + HashSet tmpAlgorithmNamesSet = new HashSet<>(tmpAlgorithmNamesSetInitCapacity, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); for (IMoleculeFragmenter tmpFragmenter : this.fragmenters) { //algorithm name should be singleton and must be persistable String tmpAlgName = tmpFragmenter.getFragmentationAlgorithmName(); if (!PreferenceUtil.isValidName(tmpAlgName) || !SingleTermPreference.isValidContent(tmpAlgName)) { throw new Exception("Algorithm name " + tmpAlgName + " is invalid."); } - if (tmpAlgorithmNames.contains(tmpAlgName)) { + if (tmpAlgorithmNamesSet.contains(tmpAlgName)) { throw new Exception("Algorithm name " + tmpAlgName + " is used multiple times."); } else { - tmpAlgorithmNames.add(tmpAlgName); + tmpAlgorithmNamesSet.add(tmpAlgName); } //setting names must be singletons within the respective class //setting names and values must adhere to the preference input restrictions //setting values are only tested for their current state, not the entire possible input space! It is tested again at persistence List tmpSettingsList = tmpFragmenter.settingsProperties(); - HashSet tmpSettingNames = new HashSet<>(tmpSettingsList.size() + 6, 1.0f); + int tmpSettingNamesSetInitCapacity = CollectionUtil.calculateInitialHashCollectionCapacity(tmpSettingsList.size(), BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); + HashSet tmpSettingNames = new HashSet<>(tmpSettingNamesSetInitCapacity, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); for (Property tmpSetting : tmpSettingsList) { if (!PreferenceUtil.isValidName(tmpSetting.getName())) { throw new Exception("Setting " + tmpSetting.getName() + " has an invalid name."); diff --git a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/FragmentationTask.java b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/FragmentationTask.java index a9ad6e90..d2931ab0 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/FragmentationTask.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/FragmentationTask.java @@ -135,7 +135,7 @@ public Integer call() throws Exception{ continue; } List tmpFragmentsOfMolList = new ArrayList<>(tmpFragmentsList.size()); - HashMap tmpFragmentFrequenciesOfMoleculeMap = new HashMap<>(CollectionUtil.calculateInitialHashMapCapacity(tmpFragmentsList.size())); + HashMap tmpFragmentFrequenciesOfMoleculeMap = new HashMap<>(CollectionUtil.calculateInitialHashCollectionCapacity(tmpFragmentsList.size())); for(IAtomContainer tmpFragment : tmpFragmentsList){ String tmpSmiles = ChemUtil.createUniqueSmiles(tmpFragment); if (tmpSmiles == null) { diff --git a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ErtlFunctionalGroupsFinderFragmenter.java b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ErtlFunctionalGroupsFinderFragmenter.java index bbc0a4b8..27886f68 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ErtlFunctionalGroupsFinderFragmenter.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ErtlFunctionalGroupsFinderFragmenter.java @@ -28,6 +28,7 @@ import de.unijena.cheminf.mortar.gui.util.GuiUtil; import de.unijena.cheminf.mortar.message.Message; import de.unijena.cheminf.mortar.model.io.Importer; +import de.unijena.cheminf.mortar.model.util.BasicDefinitions; import de.unijena.cheminf.mortar.model.util.ChemUtil; import de.unijena.cheminf.mortar.model.util.CollectionUtil; import de.unijena.cheminf.mortar.model.util.SimpleEnumConstantNameProperty; @@ -339,8 +340,8 @@ public static enum CycleFinderOption { * Constructor, all settings are initialised with their default values as declared in the respective public constants. */ public ErtlFunctionalGroupsFinderFragmenter() { - int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashMapCapacity(6, 0.75f); - this.settingNameTooltipTextMap = new HashMap(tmpInitialCapacityForSettingNameTooltipTextMap, 0.75f); + int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashCollectionCapacity(6, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); + this.settingNameTooltipTextMap = new HashMap(tmpInitialCapacityForSettingNameTooltipTextMap, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); this.fragmentSaturationSetting = new SimpleEnumConstantNameProperty(this, "Fragment saturation setting", IMoleculeFragmenter.FRAGMENT_SATURATION_OPTION_DEFAULT.name(), IMoleculeFragmenter.FragmentSaturationOption.class) { @Override @@ -807,8 +808,8 @@ public List fragmentMolecule(IAtomContainer aMolecule) this.logger.log(Level.WARNING, anException.toString(), anException); throw new IllegalArgumentException("Unexpected error at aromaticity detection: " + anException.toString()); } - int tmpInitialCapacityForIdToAtomMap = CollectionUtil.calculateInitialHashMapCapacity(tmpMoleculeClone.getAtomCount(), 0.75f); - HashMap tmpIdToAtomMap = new HashMap<>(tmpInitialCapacityForIdToAtomMap, 0.75f); + int tmpInitialCapacityForIdToAtomMap = CollectionUtil.calculateInitialHashCollectionCapacity(tmpMoleculeClone.getAtomCount(), BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); + HashMap tmpIdToAtomMap = new HashMap<>(tmpInitialCapacityForIdToAtomMap, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); for (int i = 0; i < tmpMoleculeClone.getAtomCount(); i++) { IAtom tmpAtom = tmpMoleculeClone.getAtom(i); tmpAtom.setProperty(ErtlFunctionalGroupsFinderFragmenter.INTERNAL_INDEX_PROPERTY_KEY, i); diff --git a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ScaffoldGeneratorFragmenter.java b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ScaffoldGeneratorFragmenter.java index 7fe3b5bf..f36bd65b 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ScaffoldGeneratorFragmenter.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ScaffoldGeneratorFragmenter.java @@ -22,6 +22,7 @@ import de.unijena.cheminf.mortar.gui.util.GuiUtil; import de.unijena.cheminf.mortar.message.Message; +import de.unijena.cheminf.mortar.model.util.BasicDefinitions; import de.unijena.cheminf.mortar.model.util.CollectionUtil; import de.unijena.cheminf.mortar.model.util.SimpleEnumConstantNameProperty; import de.unijena.cheminf.scaffoldGenerator.ScaffoldGenerator; @@ -341,8 +342,8 @@ public static enum FragmentationTypeOption { */ public ScaffoldGeneratorFragmenter() { this.scaffoldGeneratorInstance = new ScaffoldGenerator(); - int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashMapCapacity(16, 0.75f); - this.settingNameTooltipTextMap = new HashMap(tmpInitialCapacityForSettingNameTooltipTextMap, 0.75f); + int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashCollectionCapacity(16, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); + this.settingNameTooltipTextMap = new HashMap(tmpInitialCapacityForSettingNameTooltipTextMap, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); this.fragmentSaturationSetting = new SimpleEnumConstantNameProperty(this, "Fragment saturation setting", IMoleculeFragmenter.FRAGMENT_SATURATION_OPTION_DEFAULT.name(), IMoleculeFragmenter.FragmentSaturationOption.class) { @Override diff --git a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/SugarRemovalUtilityFragmenter.java b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/SugarRemovalUtilityFragmenter.java index 79704b92..118ed3a6 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/SugarRemovalUtilityFragmenter.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/SugarRemovalUtilityFragmenter.java @@ -28,6 +28,7 @@ import de.unijena.cheminf.deglycosylation.SugarRemovalUtility; import de.unijena.cheminf.mortar.gui.util.GuiUtil; import de.unijena.cheminf.mortar.message.Message; +import de.unijena.cheminf.mortar.model.util.BasicDefinitions; import de.unijena.cheminf.mortar.model.util.ChemUtil; import de.unijena.cheminf.mortar.model.util.CollectionUtil; import de.unijena.cheminf.mortar.model.util.SimpleEnumConstantNameProperty; @@ -236,8 +237,8 @@ public static enum SRUFragmenterReturnedFragmentsOption { public SugarRemovalUtilityFragmenter() { this.sugarRUInstance = new SugarRemovalUtility(SilentChemObjectBuilder.getInstance()); this.settings = new ArrayList<>(15); - int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashMapCapacity(20, 0.75f); - this.settingNameTooltipTextMap = new HashMap<>(tmpInitialCapacityForSettingNameTooltipTextMap, 0.75f); + int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashCollectionCapacity(20, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); + this.settingNameTooltipTextMap = new HashMap<>(tmpInitialCapacityForSettingNameTooltipTextMap, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); this.returnedFragmentsSetting = new SimpleEnumConstantNameProperty(this, "Returned fragments setting", SugarRemovalUtilityFragmenter.RETURNED_FRAGMENTS_OPTION_DEFAULT.name(), SugarRemovalUtilityFragmenter.SRUFragmenterReturnedFragmentsOption.class) { @Override diff --git a/src/main/java/de/unijena/cheminf/mortar/model/settings/SettingsContainer.java b/src/main/java/de/unijena/cheminf/mortar/model/settings/SettingsContainer.java index 18eb30af..4d0f4a3a 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/settings/SettingsContainer.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/settings/SettingsContainer.java @@ -610,8 +610,8 @@ public void reloadGlobalSettings() { * to the list of settings for display to the user. */ private void initialiseSettings() { - int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashMapCapacity(10, 0.75f); - this.settingNameTooltipTextMap = new HashMap(tmpInitialCapacityForSettingNameTooltipTextMap, 0.75f); + int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashCollectionCapacity(10, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); + this.settingNameTooltipTextMap = new HashMap(tmpInitialCapacityForSettingNameTooltipTextMap, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); this.rowsPerPageSetting = new SimpleIntegerProperty(this, "Rows per page setting", SettingsContainer.ROWS_PER_PAGE_SETTING_DEFAULT) { @@ -754,15 +754,16 @@ private void checkSettings() throws Exception { //setting names and values must adhere to the preference input restrictions //setting values are only tested for their current state, not the entire possible input space! It is tested again at persistence List tmpSettingsList = this.settings; - HashSet tmpSettingNames = new HashSet<>(tmpSettingsList.size() + 6, 1.0f); + int tmpSettingNamesSetInitCapacity = CollectionUtil.calculateInitialHashCollectionCapacity(tmpSettingsList.size(), BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); + HashSet tmpSettingNamesSet = new HashSet<>(tmpSettingNamesSetInitCapacity, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); for (Property tmpSetting : tmpSettingsList) { if (!PreferenceUtil.isValidName(tmpSetting.getName())) { throw new Exception("Setting " + tmpSetting.getName() + " has an invalid name."); } - if (tmpSettingNames.contains(tmpSetting.getName())) { + if (tmpSettingNamesSet.contains(tmpSetting.getName())) { throw new Exception("Setting name " + tmpSetting.getName() + " is used multiple times."); } else { - tmpSettingNames.add(tmpSetting.getName()); + tmpSettingNamesSet.add(tmpSetting.getName()); } if (tmpSetting instanceof SimpleBooleanProperty) { //nothing to do here, booleans cannot have invalid values diff --git a/src/main/java/de/unijena/cheminf/mortar/model/util/BasicDefinitions.java b/src/main/java/de/unijena/cheminf/mortar/model/util/BasicDefinitions.java index 274b430b..f5973925 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/util/BasicDefinitions.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/util/BasicDefinitions.java @@ -186,5 +186,9 @@ public final class BasicDefinitions { * Default distance between image and text */ public static final int DEFAULT_IMAGE_TEXT_DISTANCE = 15; + /** + * Default load factor for HashMap and HashSet instances, defined based on default value given in the Java documentation. + */ + public static final float DEFAULT_HASH_COLLECTION_LOAD_FACTOR = 0.75f; // } \ No newline at end of file diff --git a/src/main/java/de/unijena/cheminf/mortar/model/util/CollectionUtil.java b/src/main/java/de/unijena/cheminf/mortar/model/util/CollectionUtil.java index fd76826a..12073155 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/util/CollectionUtil.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/util/CollectionUtil.java @@ -111,20 +111,20 @@ public static void sortGivenFragmentListByPropertyAndSortType(ListThe initial capacity multiplied with the load factor (= rehash threshold) must be higher than the number of * elements. * - * @param aNumberOfElements number of elements supposed to be stored in the new HashMap instance - * @param aLoadFactor load factor that is specified for the new HashMap instance - * @return a suitable initial size for the new HashMap instance that leads to a rehash threshold that is slightly + * @param aNumberOfElements number of elements supposed to be stored in the new HashMap or HashSet instance + * @param aLoadFactor load factor that is specified for the new HashMap or HashSet instance + * @return a suitable initial size for the new HashMap or HashSet instance that leads to a rehash threshold that is slightly * higher than the number of elements * @throws IllegalArgumentException if the number of elements or the load factor is negative or equal to zero * or if the load factor is greater than 1.0 */ - public static int calculateInitialHashMapCapacity(int aNumberOfElements, float aLoadFactor) + public static int calculateInitialHashCollectionCapacity(int aNumberOfElements, float aLoadFactor) throws IllegalArgumentException { if (aNumberOfElements <= 0) { throw new IllegalArgumentException("Number of elements needs to be higher than 0 but is " + aNumberOfElements); @@ -137,19 +137,19 @@ public static int calculateInitialHashMapCapacity(int aNumberOfElements, float a } // /** - * Calculates a suitable initial size for instantiating a new HashMap instance based on the number of elements + * Calculates a suitable initial size for instantiating a new HashMap or HashSet instance based on the number of elements * supposed to be stored in it and the default load factor (0.75) that determines the rehash threshold. * Calculation: initCap = (int) aNumberOfElement * (1/0.75) + 2 *
The initial capacity multiplied with the load factor (= rehash threshold) must be higher than the number of * elements. * - * @param aNumberOfElements number of elements supposed to be stored in the new HashMap instance - * @return a suitable initial size for the new HashMap instance that leads to a rehash threshold that is slightly + * @param aNumberOfElements number of elements supposed to be stored in the new HashMap or HashSet instance + * @return a suitable initial size for the new HashMap or HashSet instance that leads to a rehash threshold that is slightly * higher than the number of elements * @throws IllegalArgumentException if the number of elements is negative or equal to zero */ - public static int calculateInitialHashMapCapacity(int aNumberOfElements) throws IllegalArgumentException { - return CollectionUtil.calculateInitialHashMapCapacity(aNumberOfElements, 0.75f); + public static int calculateInitialHashCollectionCapacity(int aNumberOfElements) throws IllegalArgumentException { + return CollectionUtil.calculateInitialHashCollectionCapacity(aNumberOfElements, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); } // } diff --git a/src/test/java/de/unijena/cheminf/mortar/model/util/CollectionUtilTest.java b/src/test/java/de/unijena/cheminf/mortar/model/util/CollectionUtilTest.java index 610a3340..b50b2b98 100644 --- a/src/test/java/de/unijena/cheminf/mortar/model/util/CollectionUtilTest.java +++ b/src/test/java/de/unijena/cheminf/mortar/model/util/CollectionUtilTest.java @@ -37,7 +37,7 @@ public void calculateInitialHashMapSizeTest() { float[] tmpLoadFactor = new float[]{0.75f, 0.75f, 0.6f, 0.75f}; int[] tmpExpectedInitialCapacity = new int[]{15, 135, 7255762, 42}; for (int i = 0; i < tmpNumberOfElements.length; i++) { - int tmpCalculatedInitialHashMapCapacity = CollectionUtil.calculateInitialHashMapCapacity(tmpNumberOfElements[i], tmpLoadFactor[i]); + int tmpCalculatedInitialHashMapCapacity = CollectionUtil.calculateInitialHashCollectionCapacity(tmpNumberOfElements[i], tmpLoadFactor[i]); float tmpRehashThreshold = tmpCalculatedInitialHashMapCapacity * tmpLoadFactor[i]; System.out.println("\nNumber of elements: " + tmpNumberOfElements[i]); System.out.println("Load factor: " + tmpLoadFactor[i]); From 5aae9bf360f25f25c528e079c597d5b9e1f16ece Mon Sep 17 00:00:00 2001 From: JonasSchaub Date: Tue, 2 May 2023 10:52:50 +0200 Subject: [PATCH 4/4] Adjustments according to comments, shortened doc on convenience method, declared magic numbers in local variables; --- .../ErtlFunctionalGroupsFinderFragmenter.java | 5 ++++- .../algorithm/ScaffoldGeneratorFragmenter.java | 5 ++++- .../algorithm/SugarRemovalUtilityFragmenter.java | 7 +++++-- .../mortar/model/settings/SettingsContainer.java | 5 ++++- .../mortar/model/util/CollectionUtil.java | 16 +++++++--------- 5 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ErtlFunctionalGroupsFinderFragmenter.java b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ErtlFunctionalGroupsFinderFragmenter.java index 27886f68..42089423 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ErtlFunctionalGroupsFinderFragmenter.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ErtlFunctionalGroupsFinderFragmenter.java @@ -340,7 +340,10 @@ public static enum CycleFinderOption { * Constructor, all settings are initialised with their default values as declared in the respective public constants. */ public ErtlFunctionalGroupsFinderFragmenter() { - int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashCollectionCapacity(6, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); + int tmpNumberOfSettingsForTooltipMapSize= 6; + int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashCollectionCapacity( + tmpNumberOfSettingsForTooltipMapSize, + BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); this.settingNameTooltipTextMap = new HashMap(tmpInitialCapacityForSettingNameTooltipTextMap, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); this.fragmentSaturationSetting = new SimpleEnumConstantNameProperty(this, "Fragment saturation setting", IMoleculeFragmenter.FRAGMENT_SATURATION_OPTION_DEFAULT.name(), IMoleculeFragmenter.FragmentSaturationOption.class) { diff --git a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ScaffoldGeneratorFragmenter.java b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ScaffoldGeneratorFragmenter.java index f36bd65b..cf9362ae 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ScaffoldGeneratorFragmenter.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ScaffoldGeneratorFragmenter.java @@ -342,7 +342,10 @@ public static enum FragmentationTypeOption { */ public ScaffoldGeneratorFragmenter() { this.scaffoldGeneratorInstance = new ScaffoldGenerator(); - int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashCollectionCapacity(16, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); + int tmpNumberOfSettingsForTooltipMapSize= 10; + int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashCollectionCapacity( + tmpNumberOfSettingsForTooltipMapSize, + BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); this.settingNameTooltipTextMap = new HashMap(tmpInitialCapacityForSettingNameTooltipTextMap, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); this.fragmentSaturationSetting = new SimpleEnumConstantNameProperty(this, "Fragment saturation setting", IMoleculeFragmenter.FRAGMENT_SATURATION_OPTION_DEFAULT.name(), IMoleculeFragmenter.FragmentSaturationOption.class) { diff --git a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/SugarRemovalUtilityFragmenter.java b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/SugarRemovalUtilityFragmenter.java index 118ed3a6..22c2fbef 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/SugarRemovalUtilityFragmenter.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/SugarRemovalUtilityFragmenter.java @@ -236,8 +236,11 @@ public static enum SRUFragmenterReturnedFragmentsOption { */ public SugarRemovalUtilityFragmenter() { this.sugarRUInstance = new SugarRemovalUtility(SilentChemObjectBuilder.getInstance()); - this.settings = new ArrayList<>(15); - int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashCollectionCapacity(20, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); + int tmpNumberOfSettings = 15; + this.settings = new ArrayList<>(tmpNumberOfSettings); + int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashCollectionCapacity( + tmpNumberOfSettings, + BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); this.settingNameTooltipTextMap = new HashMap<>(tmpInitialCapacityForSettingNameTooltipTextMap, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); this.returnedFragmentsSetting = new SimpleEnumConstantNameProperty(this, "Returned fragments setting", SugarRemovalUtilityFragmenter.RETURNED_FRAGMENTS_OPTION_DEFAULT.name(), SugarRemovalUtilityFragmenter.SRUFragmenterReturnedFragmentsOption.class) { diff --git a/src/main/java/de/unijena/cheminf/mortar/model/settings/SettingsContainer.java b/src/main/java/de/unijena/cheminf/mortar/model/settings/SettingsContainer.java index 4d0f4a3a..6addcbb2 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/settings/SettingsContainer.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/settings/SettingsContainer.java @@ -610,7 +610,10 @@ public void reloadGlobalSettings() { * to the list of settings for display to the user. */ private void initialiseSettings() { - int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashCollectionCapacity(10, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); + int tmpNumberOfSettings = 8; + int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashCollectionCapacity( + tmpNumberOfSettings, + BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); this.settingNameTooltipTextMap = new HashMap(tmpInitialCapacityForSettingNameTooltipTextMap, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); this.rowsPerPageSetting = new SimpleIntegerProperty(this, "Rows per page setting", diff --git a/src/main/java/de/unijena/cheminf/mortar/model/util/CollectionUtil.java b/src/main/java/de/unijena/cheminf/mortar/model/util/CollectionUtil.java index 12073155..40cd7164 100644 --- a/src/main/java/de/unijena/cheminf/mortar/model/util/CollectionUtil.java +++ b/src/main/java/de/unijena/cheminf/mortar/model/util/CollectionUtil.java @@ -112,14 +112,14 @@ public static void sortGivenFragmentListByPropertyAndSortType(ListThe initial capacity multiplied with the load factor (= rehash threshold) must be higher than the number of + *
The initial capacity multiplied with the load factor (= resize threshold) must be higher than the number of * elements. * * @param aNumberOfElements number of elements supposed to be stored in the new HashMap or HashSet instance * @param aLoadFactor load factor that is specified for the new HashMap or HashSet instance - * @return a suitable initial size for the new HashMap or HashSet instance that leads to a rehash threshold that is slightly + * @return a suitable initial size for the new HashMap or HashSet instance that leads to a resize threshold that is slightly * higher than the number of elements * @throws IllegalArgumentException if the number of elements or the load factor is negative or equal to zero * or if the load factor is greater than 1.0 @@ -137,14 +137,12 @@ public static int calculateInitialHashCollectionCapacity(int aNumberOfElements, } // /** - * Calculates a suitable initial size for instantiating a new HashMap or HashSet instance based on the number of elements - * supposed to be stored in it and the default load factor (0.75) that determines the rehash threshold. - * Calculation: initCap = (int) aNumberOfElement * (1/0.75) + 2 - *
The initial capacity multiplied with the load factor (= rehash threshold) must be higher than the number of - * elements. + * Calculates a suitable initial size for instantiating a new HashMap or HashSet instance with the default load factor. + *
For more details, see {@link #calculateInitialHashCollectionCapacity(int, float) calculateInitialHashCollectionCapacity(int, float)}. + * * * @param aNumberOfElements number of elements supposed to be stored in the new HashMap or HashSet instance - * @return a suitable initial size for the new HashMap or HashSet instance that leads to a rehash threshold that is slightly + * @return a suitable initial size for the new HashMap or HashSet instance that leads to a resize threshold that is slightly * higher than the number of elements * @throws IllegalArgumentException if the number of elements is negative or equal to zero */