Bladeren bron

Added some suggestions from the code review

Dominik Fuchß 4 jaren geleden
bovenliggende
commit
ad686abd16

+ 11 - 5
jplag.cli/src/main/java/de/jplag/CommandLineArgument.java

@@ -2,7 +2,9 @@ package de.jplag;
 
 import static de.jplag.CLI.CLUSTERING_GROUP_NAME;
 import static de.jplag.CLI.CLUSTERING_PREPROCESSING_GROUP_NAME;
-import static de.jplag.options.JPlagOptions.*;
+import static de.jplag.options.JPlagOptions.DEFAULT_COMPARISON_MODE;
+import static de.jplag.options.JPlagOptions.DEFAULT_SHOWN_COMPARISONS;
+import static de.jplag.options.JPlagOptions.DEFAULT_SIMILARITY_THRESHOLD;
 import static net.sourceforge.argparse4j.impl.Arguments.append;
 import static net.sourceforge.argparse4j.impl.Arguments.storeTrue;
 
@@ -10,15 +12,19 @@ import java.util.Collection;
 import java.util.List;
 import java.util.Optional;
 
-import net.sourceforge.argparse4j.impl.Arguments;
-import net.sourceforge.argparse4j.inf.*;
-
 import de.jplag.clustering.ClusteringAlgorithm;
 import de.jplag.clustering.ClusteringOptions;
 import de.jplag.clustering.algorithm.InterClusterSimilarity;
 import de.jplag.options.SimilarityMetric;
 import de.jplag.strategy.ComparisonMode;
 
+import net.sourceforge.argparse4j.impl.Arguments;
+import net.sourceforge.argparse4j.inf.Argument;
+import net.sourceforge.argparse4j.inf.ArgumentAction;
+import net.sourceforge.argparse4j.inf.ArgumentContainer;
+import net.sourceforge.argparse4j.inf.ArgumentParser;
+import net.sourceforge.argparse4j.inf.Namespace;
+
 /**
  * Command line arguments for the JPlag CLI. Each argument is defined through an enumeral.
  * @author Timur Saglam
@@ -27,7 +33,7 @@ public enum CommandLineArgument {
     ROOT_DIRECTORY(new Builder("rootDir", String.class).nargs(NumberOfArgumentValues.ZERO_OR_MORE_VALUES)),
     NEW_DIRECTORY(new Builder("-new", String.class).nargs(NumberOfArgumentValues.ONE_OR_MORE_VALUES)),
     OLD_DIRECTORY(new Builder("-old", String.class).nargs(NumberOfArgumentValues.ONE_OR_MORE_VALUES)),
-    LANGUAGE(new Builder("-l", String.class).defaultsTo(de.jplag.java.Language.SHORT_NAME).choices(LanguageLoader.getAllLanguageNames())),
+    LANGUAGE(new Builder("-l", String.class).defaultsTo(de.jplag.java.Language.SHORT_NAME).choices(LanguageLoader.getAllAvailableLanguageShortNames())),
     BASE_CODE("-bc", String.class),
     VERBOSITY(new Builder("-v", String.class).defaultsTo("quiet").choices(List.of("quiet", "long"))), // TODO SH: Replace verbosity when integrating a
                                                                                                       // real logging library

+ 2 - 2
jplag.cli/src/test/java/de/jplag/cli/LanguageTest.java

@@ -27,13 +27,13 @@ class LanguageTest extends CommandLineInterfaceTest {
 
     @Test
     void testLoading() {
-        var languages = LanguageLoader.loadLanguages();
+        var languages = LanguageLoader.getAllAvailableLanguages();
         Assertions.assertEquals(9, languages.size());
     }
 
     @Test
     void testValidLanguages() {
-        for (Language language : LanguageLoader.loadLanguages()) {
+        for (Language language : LanguageLoader.getAllAvailableLanguages()) {
             String argument = buildArgument(CommandLineArgument.LANGUAGE, language.getShortName());
             buildOptionsFromCLI(argument, CURRENT_DIRECTORY);
             assertEquals(language.getShortName(), options.getLanguageShortName());

+ 24 - 21
jplag.frontend-utils/src/main/java/de/jplag/LanguageLoader.java

@@ -1,6 +1,12 @@
 package de.jplag;
 
-import java.util.*;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.ServiceLoader;
+import java.util.TreeMap;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -12,7 +18,7 @@ import org.slf4j.LoggerFactory;
 public final class LanguageLoader {
     private static final Logger logger = LoggerFactory.getLogger(LanguageLoader.class);
 
-    private static List<Language> loaded = null;
+    private static Map<String,Language> loaded = null;
 
     private LanguageLoader() {
         throw new IllegalAccessError();
@@ -22,28 +28,25 @@ public final class LanguageLoader {
      * Load all languages that are currently in the classpath.
      * @return the languages
      */
-    public static synchronized List<Language> loadLanguages() {
+    public static synchronized List<Language> getAllAvailableLanguages() {
         if (loaded != null)
-            return loaded;
+            return loaded.values().stream().toList();
 
-        Set<String> loadedShortNames = new HashSet<>();
-        List<Language> languages = new ArrayList<>();
+        Map<String, Language> languages = new TreeMap<>();
 
-        for (Language notInitializedLanguage : ServiceLoader.load(Language.class)) {
-            String shortName = notInitializedLanguage.getShortName();
-            if (!loadedShortNames.add(shortName)) {
+        for (Language language : ServiceLoader.load(Language.class)) {
+            String shortName = language.getShortName();
+            if (languages.containsKey(shortName)) {
                 logger.error("Multiple implementations for a language '{}' are present in the classpath! Skipping ..", shortName);
-                languages.removeIf(l -> Objects.equals(shortName, l.getShortName()));
+                languages.remove(shortName);
                 continue;
             }
-            logger.info("Loading Language Frontend '{}'", notInitializedLanguage.getName());
-            languages.add(notInitializedLanguage);
+            logger.info("Loading Language Frontend '{}'", language.getName());
+            languages.put(shortName, language);
         }
 
-        languages.sort(Comparator.comparing(Language::getShortName));
-
-        loaded = Collections.unmodifiableList(languages);
-        return loaded;
+        loaded = Collections.unmodifiableMap(languages);
+        return loaded.values().stream().toList();
     }
 
     /**
@@ -51,8 +54,8 @@ public final class LanguageLoader {
      * @param shortName the short name of the language
      * @return the language or an empty optional if no language has been found.
      */
-    public static Optional<Language> loadLanguage(String shortName) {
-        var result = loadLanguages().stream().filter(it -> Objects.equals(it.getShortName(), shortName)).findFirst();
+    public static Optional<Language> getLanguage(String shortName) {
+        var result = getAllAvailableLanguages().stream().filter(it -> Objects.equals(it.getShortName(), shortName)).findFirst();
         if (result.isEmpty())
             logger.warn("Attempt to load Language {} was not successful", shortName);
         return result;
@@ -62,14 +65,14 @@ public final class LanguageLoader {
      * Get a list of all available languages with their short name.
      * @return the list of all languages
      */
-    public static List<String> getAllLanguageNames() {
-        return loadLanguages().stream().map(Language::getShortName).toList();
+    public static List<String> getAllAvailableLanguageShortNames() {
+        return getAllAvailableLanguages().stream().map(Language::getShortName).toList();
     }
 
     /**
      * Resets the internal cache of all loaded languages
      */
-    public static synchronized void reload() {
+    public static synchronized void clearCache() {
         loaded = null;
     }
 }

+ 1 - 1
jplag/src/main/java/de/jplag/JPlag.java

@@ -113,7 +113,7 @@ public class JPlag {
             return currentLanguage;
         }
 
-        Language language = LanguageLoader.loadLanguage(languageShortName).orElseThrow();
+        Language language = LanguageLoader.getLanguage(languageShortName).orElseThrow();
         options.setLanguage(language);
         options.setLanguageDefaults(language);
         logger.info("Loaded language {}", language.getName());

+ 8 - 4
jplag/src/test/java/de/jplag/special/TokenPrinterTest.java

@@ -7,7 +7,11 @@ import java.util.function.Consumer;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
-import de.jplag.*;
+import de.jplag.JPlagResult;
+import de.jplag.LanguageLoader;
+import de.jplag.Submission;
+import de.jplag.TestBase;
+import de.jplag.TokenPrinter;
 import de.jplag.exceptions.ExitException;
 import de.jplag.options.JPlagOptions;
 
@@ -31,7 +35,7 @@ public class TokenPrinterTest extends TestBase {
     @Test
     void printCPPFiles() {
         printSubmissions(options -> {
-            options.setLanguage(LanguageLoader.loadLanguage(LANGUAGE_CPP).orElseThrow());
+            options.setLanguage(LanguageLoader.getLanguage(LANGUAGE_CPP).orElseThrow());
             options.setMinimumTokenMatch(MIN_TOKEN_MATCH); // for printing also allow small files
         });
     }
@@ -48,7 +52,7 @@ public class TokenPrinterTest extends TestBase {
     @Test
     void printRLangFiles() {
         printSubmissions(options -> {
-            options.setLanguage(LanguageLoader.loadLanguage(LANGUAGE_R).orElseThrow());
+            options.setLanguage(LanguageLoader.getLanguage(LANGUAGE_R).orElseThrow());
             options.setMinimumTokenMatch(MIN_TOKEN_MATCH); // for printing also allow small files
         });
     }
@@ -56,7 +60,7 @@ public class TokenPrinterTest extends TestBase {
     @Disabled("Not a meaningful test, used for designing the token set")
     @Test
     void printKotlinFiles() {
-        printSubmissions(options -> options.setLanguage(LanguageLoader.loadLanguage(LANGUAGE_KOTLIN).orElseThrow()));
+        printSubmissions(options -> options.setLanguage(LanguageLoader.getLanguage(LANGUAGE_KOTLIN).orElseThrow()));
     }
 
     private void printSubmissions(Consumer<JPlagOptions> optionsCustomization) {