Merge pull request #363 from hannobraun/model

Clean up `Model`

src/main.rs

@@ -53,10 +53,11 @@ fn main() -> anyhow::Result<()> {
 
     let args = Args::parse();
     let config = Config::load()?;
-    let model = Model::new(
-        config.default_path,
-        args.model.unwrap_or(config.default_model),
-    );
+
+    let mut path = config.default_path;
+    path.push(args.model.unwrap_or(config.default_model));
+
+    let model = Model::from_path(path)?;
 
     let mut parameters = HashMap::new();
     for parameter in args.parameters {

src/model.rs

@@ -1,63 +1,57 @@
-use std::{
-    collections::HashMap,
-    io,
-    path::{Path, PathBuf},
-    process::Command,
-};
+use std::{collections::HashMap, io, path::PathBuf, process::Command};
 
 use thiserror::Error;
 
 pub struct Model {
-    path: PathBuf,
+    src_path: PathBuf,
+    lib_path: PathBuf,
+    manifest_path: PathBuf,
 }
 
 impl Model {
-    pub fn new(base_path: PathBuf, rel_path: PathBuf) -> Self {
-        let mut path = base_path;
-        path.push(rel_path);
+    pub fn from_path(path: PathBuf) -> io::Result<Self> {
+        let name = {
+            // Can't panic. It only would, if the path ends with "..", and we
+            // are canonicalizing it here to prevent that.
+            let canonical = path.canonicalize()?;
+            let file_name = canonical.file_name().unwrap();
+
+            file_name.to_string_lossy().replace('-', "_")
+        };
 
-        Self { path }
-    }
+        let src_path = path.join("src");
 
-    pub fn name(&self) -> Result<String, io::Error> {
-        let canonical = self.path.canonicalize()?;
+        let lib_path = {
+            let file = if cfg!(windows) {
+                format!("{}.dll", name)
+            } else if cfg!(target_os = "macos") {
+                format!("lib{}.dylib", name)
+            } else {
+                //Unix
+                format!("lib{}.so", name)
+            };
 
-        // Can't panic. It only would, if the path ends with "..", and we just
-        // canonicalized it.
-        let file_name = canonical.file_name().unwrap();
+            path.join("target/debug").join(file)
+        };
 
-        Ok(file_name.to_string_lossy().into_owned())
-    }
+        let manifest_path = path.join("Cargo.toml");
 
-    pub fn path(&self) -> &Path {
-        &self.path
+        Ok(Self {
+            src_path,
+            lib_path,
+            manifest_path,
+        })
     }
 
     pub fn src_path(&self) -> PathBuf {
-        self.path().join("src")
-    }
-
-    pub fn lib_path(&self) -> Result<PathBuf, io::Error> {
-        let name = self.name()?.replace('-', "_");
-
-        let file = if cfg!(windows) {
-            format!("{}.dll", name)
-        } else if cfg!(target_os = "macos") {
-            format!("lib{}.dylib", name)
-        } else {
-            //Unix
-            format!("lib{}.so", name)
-        };
-
-        Ok(self.path().join("target/debug").join(file))
+        self.src_path.clone()
     }
 
     pub fn load(
         &self,
         arguments: &HashMap<String, String>,
     ) -> Result<fj::Shape, Error> {
-        let manifest_path =
-            self.path().join("Cargo.toml").display().to_string();
+        let manifest_path = self.manifest_path.display().to_string();
 
         let status = Command::new("cargo")
             .arg("build")
@@ -85,7 +79,7 @@ impl Model {
         // to switch to a better technique:
         // https://github.com/hannobraun/Fornjot/issues/71
         let shape = unsafe {
-            let lib = libloading::Library::new(self.lib_path()?)?;
+            let lib = libloading::Library::new(&self.lib_path)?;
             let model: libloading::Symbol<ModelFn> = lib.get(b"model")?;
             model(arguments)
         };