june 2024 code improvements (#44)

june 2024 code improvements

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "osm_extract_polygon"
-version = "0.5.3"
+version = "0.5.4"
 authors = ["Andreas <andreas.gemsa@googlemail.com>"]
 edition = "2018"
 

src/output/file_creator.rs

@@ -17,22 +17,22 @@ enum OverwriteOrSkip {
 
 impl FileCreator {
     pub fn create_file(&mut self, filename: &str) -> std::io::Result<File> {
-        let file_exists = Path::new(&filename).exists();
-        if file_exists {
-            let overwrite_mode = &self.overwrite_handling(filename);
-            if let OverwriteOrSkip::Skip = overwrite_mode {
-                //Note: this is not nice since it returns an error in a normal user flow
-                return Err(Error::new(ErrorKind::AlreadyExists, "skipped"));
+        if Path::new(filename).exists() {
+            match self.overwrite_handling(filename)? {
+                OverwriteOrSkip::Skip => {
+                    return Err(Error::new(ErrorKind::AlreadyExists, "skipped"));
+                }
+                OverwriteOrSkip::Overwrite => {}
             }
         }
 
         File::create(filename)
     }
 
-    fn overwrite_handling(&mut self, filename: &str) -> OverwriteOrSkip {
-        match &self.overwrite_mode_config {
-            OverwriteConfiguration::OverwriteAll => return OverwriteOrSkip::Overwrite,
-            OverwriteConfiguration::SkipAll => return OverwriteOrSkip::Skip,
+    fn overwrite_handling(&mut self, filename: &str) -> io::Result<OverwriteOrSkip> {
+        match self.overwrite_mode_config {
+            OverwriteConfiguration::OverwriteAll => return Ok(OverwriteOrSkip::Overwrite),
+            OverwriteConfiguration::SkipAll => return Ok(OverwriteOrSkip::Skip),
             _ => {}
         }
 
@@ -42,19 +42,21 @@ impl FileCreator {
 
             io::stdin().read_line(&mut buffer).expect("Couldn't read line");
 
-            buffer = String::from(buffer.trim());
-
-            if buffer.as_str() == "k" {
-                self.overwrite_mode_config = OverwriteConfiguration::SkipAll;
-            } else if buffer.as_str() == "a" {
-                self.overwrite_mode_config = OverwriteConfiguration::OverwriteAll;
-            }
+            let input = buffer.trim();
 
-            match buffer.as_str() {
-                "s" | "k" => return OverwriteOrSkip::Skip,
-                "o" | "a" => return OverwriteOrSkip::Overwrite,
+            match input {
+                "s" => return Ok(OverwriteOrSkip::Skip),
+                "o" => return Ok(OverwriteOrSkip::Overwrite),
+                "k" => {
+                    self.overwrite_mode_config = OverwriteConfiguration::SkipAll;
+                    return Ok(OverwriteOrSkip::Skip);
+                }
+                "a" => {
+                    self.overwrite_mode_config = OverwriteConfiguration::OverwriteAll;
+                    return Ok(OverwriteOrSkip::Overwrite);
+                }
                 _ => {
-                    buffer = String::from("");
+                    buffer.clear();
                 }
             }
         }

src/output/file_writer_geojson.rs

@@ -15,12 +15,9 @@ pub struct GeoJsonWriter {}
 
 impl FileWriter for GeoJsonWriter {
     fn write_to_file(&self, file: &mut File, polygon: &Polygon) -> std::io::Result<()> {
-        let feature = convert_polygon_to_geojson_feature(polygon);
-        if feature.is_ok() {
-            Ok(file.write_all(feature.unwrap().to_string().as_bytes())?)
-        } else {
-            Err(Error::new(ErrorKind::Other, "Error in converting Polygon to GeoJSON."))
-        }
+        let feature = convert_polygon_to_geojson_feature(polygon)
+            .map_err(|_| Error::new(ErrorKind::Other, "Error in converting Polygon to GeoJSON"))?;
+        file.write_all(feature.to_string().as_bytes())
     }
 }
 

src/output/file_writer_poly.rs

@@ -1,7 +1,6 @@
 use crate::converter::Polygon;
 use crate::output::output_handler::FileWriter;
 
-use std::fmt;
 use std::fs::File;
 use std::io::prelude::*;
 
@@ -13,17 +12,16 @@ impl FileWriter for PolyWriter {
         output.push_str(&polygon.name);
         output.push('\n');
 
-        let mut index: i32 = 1;
-
-        for points in &polygon.points {
-            let area_id_str = fmt::format(format_args!("area_{}\n", index));
+        for (index, points) in polygon.points.iter().enumerate() {
+            let area_id_str = format!("area_{}\n", index + 1);
             output.push_str(&area_id_str);
+
             for point in points {
-                let point_str = fmt::format(format_args!("\t{} \t{}\n", point.lon, point.lat));
+                let point_str = format!("\t{} \t{}\n", point.lon, point.lat);
                 output.push_str(&point_str);
             }
+
             output.push_str("END\n");
-            index += 1;
         }
         output.push_str("END\n");
         file.write_all(output.as_bytes())?;

src/output/output_handler.rs

@@ -4,26 +4,24 @@ use crate::output::file_writer_geojson::GeoJsonWriter;
 use crate::output::file_writer_poly::PolyWriter;
 use crate::output::OverwriteConfiguration;
 
-use std::fs::File;
-use std::time::Instant;
-
 use std::collections::HashSet;
-use std::fs::create_dir_all;
+use std::fs::{create_dir_all, File};
+use std::io::Result;
+use std::time::Instant;
 
 pub trait FileWriter {
-    fn write_to_file(&self, file: &mut File, polygon: &Polygon) -> std::io::Result<()>;
+    fn write_to_file(&self, file: &mut File, polygon: &Polygon) -> Result<()>;
 }
 
 pub struct OutputHandlerConfiguration {
     pub overwrite_configuration: OverwriteConfiguration,
     pub geojson_output: bool,
 }
 
-pub fn write(folder: &str, polygons: &[Polygon], config: OutputHandlerConfiguration) -> std::io::Result<u64> {
+pub fn write(folder: &str, polygons: &[Polygon], config: OutputHandlerConfiguration) -> Result<u64> {
     create_dir_all(folder)?;
 
     let filename_polys = pair_safe_filenames_and_polygons(polygons);
-
     let mut output_handler = new_output_handler(config);
 
     output_handler.write_files(folder, filename_polys)
@@ -46,8 +44,8 @@ struct OutputHandler {
 }
 
 impl OutputHandler {
-    pub fn write_files(&mut self, base_folder: &str, filename_polys: Vec<(String, &Polygon)>) -> std::io::Result<u64> {
-        let mut file_count: u64 = 0;
+    pub fn write_files(&mut self, base_folder: &str, filename_polys: Vec<(String, &Polygon)>) -> Result<u64> {
+        let mut file_count = 0;
 
         let poly_writer = PolyWriter {};
         let geojson_writer = GeoJsonWriter {};
@@ -57,22 +55,15 @@ impl OutputHandler {
 
         for (name, polygon) in filename_polys {
             let filename_wo_ext = format!("{}/{}", base_folder, name);
-            if self.write_poly {
-                let success_poly = self.write_file(&filename_wo_ext, "poly", polygon, &poly_writer);
-                if success_poly {
-                    file_count += 1;
-                }
+            if self.write_poly && self.write_file(&filename_wo_ext, "poly", polygon, &poly_writer) {
+                file_count += 1;
             }
-            if self.write_geojson {
-                let success_geojson = self.write_file(&filename_wo_ext, "geojson", polygon, &geojson_writer);
-                if success_geojson {
-                    file_count += 1;
-                }
+            if self.write_geojson && self.write_file(&filename_wo_ext, "geojson", polygon, &geojson_writer) {
+                file_count += 1;
             }
         }
 
         println!("finished writing! {}s", now.elapsed().as_secs());
-
         Ok(file_count)
     }
 
@@ -105,37 +96,32 @@ impl OutputHandler {
 
 fn pair_safe_filenames_and_polygons(polygons: &[Polygon]) -> Vec<(String, &Polygon)> {
     let safe_names: Vec<String> = polygons.iter().map(|p| make_safe(&p.name)).collect();
-
     let mut seen_names: HashSet<String> = HashSet::new();
     let mut duplicate_names: HashSet<String> = HashSet::new();
 
-    safe_names.iter().for_each(|name| {
-        if seen_names.contains(&name.to_lowercase()) {
-            duplicate_names.insert(name.to_string().to_lowercase());
-        } else {
-            seen_names.insert(name.to_string().to_lowercase());
+    for name in &safe_names {
+        let lower_name = name.to_lowercase();
+        if !seen_names.insert(lower_name.clone()) {
+            duplicate_names.insert(lower_name);
         }
-    });
+    }
 
-    safe_names
+    polygons
         .iter()
-        .zip(polygons.iter())
-        .map(|(name, p)| {
+        .zip(safe_names)
+        .map(|(p, name)| {
             let out_name = if duplicate_names.contains(&name.to_lowercase()) {
                 format!("{}_{}", name, p.relation_id)
             } else {
-                name.to_string()
+                name
             };
-
             (out_name, p)
         })
         .collect()
 }
 
 fn make_safe(name: &str) -> String {
-    let mut s = String::from(name);
-    s.retain(|c| !r"\\/&:<>|*".contains(c));
-    s
+    name.chars().filter(|c| !r"\\/&:<>|*".contains(*c)).collect()
 }
 
 // ////////////////////////////////////

src/utils.rs

@@ -1,11 +1,9 @@
 use std::collections::{HashMap, HashSet};
 use std::hash::Hash;
-use std::cmp::Eq;
-use std::clone::Clone;
 
 pub fn values_to_set<K, V>(map: &HashMap<K, Vec<V>>) -> HashSet<V>
 where
     V: Hash + Eq + Clone,
 {
-    map.values().flat_map(|v| v.clone()).collect()
-}
+    map.values().flat_map(|v| v.iter().cloned()).collect()
+}