Merge pull request #778 from lonvia/clean-code-smells

Clean up comments and fix some code smells

src/main/java/de/komoot/photon/App.java

@@ -17,7 +17,9 @@
 
 import static spark.Spark.*;
 
-
+/**
+ * Main Photon application.
+ */
 public class App {
     private static final Logger LOGGER = org.slf4j.LoggerFactory.getLogger(App.class);
 
@@ -59,7 +61,7 @@ public static void main(String[] rawArgs) throws Exception {
                 return;
             }
 
-            // no special action specified -> normal mode: start search API
+            // No special action specified -> normal mode: start search API
             startApi(args, esServer);
         } finally {
             if (shutdownES) esServer.shutdown();
@@ -78,7 +80,7 @@ private static CommandLineArgs parseCommandLine(String[] rawArgs) {
                 throw new ParameterException("Use only one cors configuration type");
             }
         } catch (ParameterException e) {
-            LOGGER.warn("could not start photon: " + e.getMessage());
+            LOGGER.warn("Could not start photon: {}", e.getMessage());
             jCommander.usage();
             System.exit(1);
         }
@@ -94,9 +96,7 @@ private static CommandLineArgs parseCommandLine(String[] rawArgs) {
 
 
     /**
-     * take nominatim data and dump it to json
-     *
-     * @param args
+     * Take nominatim data and dump it to a Json file.
      */
     private static void startJsonDump(CommandLineArgs args) {
         try {
@@ -105,9 +105,9 @@ private static void startJsonDump(CommandLineArgs args) {
             NominatimConnector nominatimConnector = new NominatimConnector(args.getHost(), args.getPort(), args.getDatabase(), args.getUser(), args.getPassword());
             nominatimConnector.setImporter(jsonDumper);
             nominatimConnector.readEntireDatabase(args.getCountryCodes());
-            LOGGER.info("json dump was created: " + filename);
+            LOGGER.info("Json dump was created: {}", filename);
         } catch (FileNotFoundException e) {
-            LOGGER.error("cannot create dump", e);
+            LOGGER.error("Cannot create dump", e);
         }
     }
 
@@ -120,15 +120,15 @@ private static void startNominatimImport(CommandLineArgs args, Server esServer)
         try {
             dbProperties = esServer.recreateIndex(args.getLanguages()); // clear out previous data
         } catch (IOException e) {
-            throw new RuntimeException("cannot setup index, elastic search config files not readable", e);
+            throw new RuntimeException("Cannot setup index, elastic search config files not readable", e);
         }
 
-        LOGGER.info("starting import from nominatim to photon with languages: " + String.join(",", dbProperties.getLanguages()));
+        LOGGER.info("Starting import from nominatim to photon with languages: {}", String.join(",", dbProperties.getLanguages()));
         NominatimConnector nominatimConnector = new NominatimConnector(args.getHost(), args.getPort(), args.getDatabase(), args.getUser(), args.getPassword());
         nominatimConnector.setImporter(esServer.createImporter(dbProperties.getLanguages(), args.getExtraTags()));
         nominatimConnector.readEntireDatabase(args.getCountryCodes());
 
-        LOGGER.info("imported data from nominatim to photon with languages: " + String.join(",", dbProperties.getLanguages()));
+        LOGGER.info("Imported data from nominatim to photon with languages: {}", String.join(",", dbProperties.getLanguages()));
     }
 
     private static void startNominatimUpdateInit(CommandLineArgs args) {
@@ -137,9 +137,9 @@ private static void startNominatimUpdateInit(CommandLineArgs args) {
     }
 
 
-        /**
-         * Prepare Nominatim updater.
-         */
+    /**
+     * Prepare Nominatim updater.
+     */
     private static NominatimUpdater setupNominatimUpdater(CommandLineArgs args, Server server) {
         // Get database properties and ensure that the version is compatible.
         DatabaseProperties dbProperties = new DatabaseProperties();
@@ -168,9 +168,8 @@ private static void startApi(CommandLineArgs args, Server server) {
         if (allowedOrigin != null) {
             CorsFilter.enableCORS(allowedOrigin, "get", "*");
         } else {
-            before((request, response) -> {
-                response.type("application/json; charset=UTF-8"); // in the other case set by enableCors
-            });
+            // Set Json content type. In the other case already set by enableCors.
+            before((request, response) -> response.type("application/json; charset=UTF-8"));
         }
 
         // setup search API
@@ -187,7 +186,7 @@ private static void startApi(CommandLineArgs args, Server server) {
             // setup update API
             final NominatimUpdater nominatimUpdater = setupNominatimUpdater(args, server);
             get("/nominatim-update", (Request request, Response response) -> {
-                new Thread(() -> nominatimUpdater.update()).start();
+                new Thread(nominatimUpdater::update).start();
                 return "nominatim update started (more information in console output) ...";
             });
         }

src/main/java/de/komoot/photon/CommandLineArgs.java

@@ -1,87 +1,85 @@
 package de.komoot.photon;
 
-/**
- * Command Line Arguments parsed by {@link com.beust.jcommander.JCommander} and used to start photon.
- */
-
 import com.beust.jcommander.Parameter;
 import de.komoot.photon.utils.StringArrayConverter;
 
 import java.io.File;
 
-
+/**
+ * Command Line Arguments parsed by {@link com.beust.jcommander.JCommander} and used to start photon.
+ */
 public class CommandLineArgs {
 
-    @Parameter(names = "-cluster", description = "name of elasticsearch cluster to put the server into (default is 'photon')")
+    @Parameter(names = "-cluster", description = "Name of ElasticSearch cluster to put the server into")
     private String cluster = "photon";
 
-    @Parameter(names = "-transport-addresses", description = "the comma separated addresses of external elasticsearch nodes where the client can connect to (default is an empty string which forces an internal node to start)", converter = StringArrayConverter.class)
+    @Parameter(names = "-transport-addresses", description = "Comma-separated list of addresses of external ElasticSearch nodes the client can connect to (default is an empty string which forces an internal node to start)", converter = StringArrayConverter.class)
     private String[] transportAddresses = new String[]{};
 
-    @Parameter(names = "-nominatim-import", description = "import nominatim database into photon (this will delete previous index)")
+    @Parameter(names = "-nominatim-import", description = "Import nominatim database into photon (this will delete previous index)")
     private boolean nominatimImport = false;
 
-    @Parameter(names = "-nominatim-update-init-for", description = "set up tracking of updates in the Nominatim database for the given user and exit")
+    @Parameter(names = "-nominatim-update-init-for", description = "Set up tracking of updates in the Nominatim database for the given user and exit")
     private String nominatimUpdateInit = null;
 
-    @Parameter(names = "-nominatim-update", description = "fetch updates from nominatim database into photon and exit (this updates the index only without offering an API)")
+    @Parameter(names = "-nominatim-update", description = "Fetch updates from nominatim database into photon and exit (updates the index only without offering an API)")
     private boolean nominatimUpdate = false;
 
-    @Parameter(names = "-languages", description = "languages nominatim importer should import and use at run-time, comma separated (default is 'en,fr,de,it')", converter = StringArrayConverter.class)
+    @Parameter(names = "-languages", description = "[import-only] Comma-separated list of languages for which names should be imported (default is 'en,fr,de,it')", converter = StringArrayConverter.class)
     private String[] languages = new String[]{};
 
-    @Parameter(names = "-default-language", description = "language to return results in when no explicit language is choosen by the user")
+    @Parameter(names = "-default-language", description = "Language to return results in when no explicit language is chosen by the user")
     private String defaultLanguage = "default";
 
-    @Parameter(names = "-country-codes", description = "country codes filter that nominatim importer should import, comma separated. If empty full planet is done", converter = StringArrayConverter.class)
+    @Parameter(names = "-country-codes", description = "[import-only] Comma-separated list of country codes for countries the importer should import, comma separated (default is empty which imports the full database)", converter = StringArrayConverter.class)
     private String[] countryCodes = new String[]{};
 
-    @Parameter(names = "-extra-tags", description = "comma-separated list of additional tags to save for each place", converter = StringArrayConverter.class)
+    @Parameter(names = "-extra-tags", description = "Comma-separated list of additional tags to save for each place (default: None)", converter = StringArrayConverter.class)
     private String[] extraTags = new String[]{};
 
-    @Parameter(names = "-synonym-file", description = "file with synonym and classification terms")
+    @Parameter(names = "-synonym-file", description = "File with synonym and classification terms")
     private String synonymFile = null;
 
     @Parameter(names = "-query-timeout", description = "Time after which to cancel queries to the ES database (in seconds).")
     private int queryTimeout = 7;
 
-    @Parameter(names = "-json", description = "import nominatim database and dump it to a json like files in (useful for developing)")
+    @Parameter(names = "-json", description = "Read from nominatim database and dump it to the given file in a json-like format (useful for developing)")
     private String jsonDump = null;
 
-    @Parameter(names = "-host", description = "postgres host (default 127.0.0.1)")
+    @Parameter(names = "-host", description = "Hostname of the PostgreSQL database")
     private String host = "127.0.0.1";
 
-    @Parameter(names = "-port", description = "postgres port (default 5432)")
+    @Parameter(names = "-port", description = "Port of the PostgreSQL database")
     private Integer port = 5432;
 
-    @Parameter(names = "-database", description = "postgres host (default nominatim)")
+    @Parameter(names = "-database", description = "Database name of the Nominatim database")
     private String database = "nominatim";
 
-    @Parameter(names = "-user", description = "postgres user (default nominatim)")
+    @Parameter(names = "-user", description = "Username in the PostgreSQL database")
     private String user = "nominatim";
 
-    @Parameter(names = "-password", description = "postgres password (default '')")
+    @Parameter(names = "-password", description = "Password for the PostgreSQL database")
     private String password = null;
 
-    @Parameter(names = "-data-dir", description = "data directory (default '.')")
+    @Parameter(names = "-data-dir", description = "Photon data directory")
     private String dataDirectory = new File(".").getAbsolutePath();
 
-    @Parameter(names = "-listen-port", description = "listen to port (default 2322)")
+    @Parameter(names = "-listen-port", description = "Port for the Photon server to listen to")
     private int listenPort = 2322;
 
-    @Parameter(names = "-listen-ip", description = "listen to address (default '0.0.0.0')")
+    @Parameter(names = "-listen-ip", description = "Address for the Photon server to listen to")
     private String listenIp = "0.0.0.0";
 
-    @Parameter(names = "-cors-any", description = "enable cross-site resource sharing for any origin ((default CORS not supported)")
+    @Parameter(names = "-cors-any", description = "Enable cross-site resource sharing for any origin")
     private boolean corsAnyOrigin = false;
 
-    @Parameter(names = "-cors-origin", description = "enable cross-site resource sharing for the specified origin (default CORS not supported)")
+    @Parameter(names = "-cors-origin", description = "Enable cross-site resource sharing for the specified origin")
     private String corsOrigin = null;
 
     @Parameter(names = "-enable-update-api", description = "Enable the additional endpoint /nominatim-update, which allows to trigger updates from a nominatim database")
     private boolean enableUpdateApi = false;
 
-    @Parameter(names = "-h", description = "show help / usage")
+    @Parameter(names = "-h", description = "Show help / usage")
     private boolean usage = false;
 
     public String[] getLanguages(boolean useDefaultIfEmpty) {

src/main/java/de/komoot/photon/Constants.java

@@ -1,9 +1,7 @@
 package de.komoot.photon;
 
 /**
- * date: 17.10.14
- *
- * @author christoph
+ * Various string constants.
  */
 public class Constants {
     public static final String POSTCODE = "postcode";
@@ -17,14 +15,8 @@ public class Constants {
     public static final String STREET = "street";
     public static final String STATE = "state";
     public static final String COUNTY = "county";
-    public static final String TYPE = "type";
-    public static final String GEOMETRY = "geometry";
-    public static final String PROPERTIES = "properties";
-    public static final String COORDINATES = "coordinates";
     public static final String LAT = "lat";
     public static final String LON = "lon";
-    public static final String FEATURE = "Feature";
-    public static final String POINT = "Point";
     public static final String IMPORTANCE = "importance";
     public static final String OSM_ID = "osm_id";
     public static final String OSM_TYPE = "osm_type";

src/main/java/de/komoot/photon/DatabaseProperties.java

@@ -5,7 +5,7 @@
 /**
  * Class collecting database global properties.
  *
- * The server is responsible for making the data persistent throught the Photon database.
+ * The server is responsible for making the data persistent in the Photon database.
  */
 public class DatabaseProperties {
     private String[] languages = null;
@@ -27,7 +27,7 @@ public String[] getLanguages() {
      *
      * @param languages Array of two-letter language codes.
      *
-     * @return This object for chaining.
+     * @return This object for function chaining.
      */
     public DatabaseProperties setLanguages(String[] languages) {
         this.languages = languages;
@@ -57,8 +57,8 @@ public void restrictLanguages(String[] languageList) {
             }
 
             if (newLanguageList.isEmpty()) {
-                throw new RuntimeException("Language list '" + languageList.toString() +
-                        "not compatible with languages in database(" + languages.toString() + ")");
+                throw new RuntimeException("Language list '" + Arrays.toString(languageList) +
+                        "' not compatible with languages in database(" + Arrays.toString(languages) + ")");
             }
 
             languages = newLanguageList.toArray(new String[]{});

src/main/java/de/komoot/photon/Importer.java

@@ -1,20 +1,16 @@
 package de.komoot.photon;
 
 /**
- * interface for bulk imports from a data source like nominatim
- *
- * @author felix
+ * Interface for bulk imports from a data source like nominatim
  */
 public interface Importer {
     /**
-     * a new document was imported
-     *
-     * @param doc
+     * Add a new document to the Photon database.
      */
-    public void add(PhotonDoc doc, int object_id);
+    public void add(PhotonDoc doc, int objectId);
 
     /**
-     * import is finished
+     * Finish up the import.
      */
     public void finish();
 }

src/main/java/de/komoot/photon/JsonDumper.java

@@ -7,7 +7,7 @@
 import java.io.PrintWriter;
 
 /**
- * useful to create json files that can be used for fast re imports
+ * Importer which writes out the documents in a json-like file.
  */
 public class JsonDumper implements Importer {
     private static final Logger LOGGER = org.slf4j.LoggerFactory.getLogger(JsonDumper.class);
@@ -23,12 +23,12 @@ public JsonDumper(String filename, String[] languages, String[] extraTags) throw
     }
 
     @Override
-    public void add(PhotonDoc doc, int object_id) {
+    public void add(PhotonDoc doc, int objectId) {
         try {
             writer.println("{\"index\": {}}");
             writer.println(Utils.convert(doc, languages, extraTags).string());
         } catch (IOException e) {
-            LOGGER.error("error writing json file", e);
+            LOGGER.error("Error writing json file", e);
         }
     }