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,
+    let mut path = config.default_path;
-        args.model.unwrap_or(config.default_model),
+    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,45 +1,27 @@
-use std::{
+use std::{collections::HashMap, io, path::PathBuf, process::Command};
-    collections::HashMap,
-    io,
-    path::{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 {
+    pub fn from_path(path: PathBuf) -> io::Result<Self> {
-        let mut path = base_path;
+        let name = {
-        path.push(rel_path);
+            // Can't panic. It only would, if the path ends with "..", and we
-
+            // are canonicalizing it here to prevent that.
-        Self { path }
+            let canonical = path.canonicalize()?;
-    }
-
-    pub fn name(&self) -> Result<String, io::Error> {
-        let canonical = self.path.canonicalize()?;
-
-        // Can't panic. It only would, if the path ends with "..", and we just
-        // canonicalized it.
             let file_name = canonical.file_name().unwrap();
 
-        Ok(file_name.to_string_lossy().into_owned())
+            file_name.to_string_lossy().replace('-', "_")
-    }
+        };
 
-    pub fn path(&self) -> &Path {
+        let src_path = path.join("src");
-        &self.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 lib_path = {
             let file = if cfg!(windows) {
                 format!("{}.dll", name)
             } else if cfg!(target_os = "macos") {
@@ -49,15 +31,27 @@ impl Model {
                 format!("lib{}.so", name)
             };
 
-        Ok(self.path().join("target/debug").join(file))
+            path.join("target/debug").join(file)
+        };
+
+        let manifest_path = path.join("Cargo.toml");
+
+        Ok(Self {
+            src_path,
+            lib_path,
+            manifest_path,
+        })
+    }
+
+    pub fn src_path(&self) -> PathBuf {
+        self.src_path.clone()
     }
 
     pub fn load(
         &self,
         arguments: &HashMap<String, String>,
     ) -> Result<fj::Shape, Error> {
-        let manifest_path =
+        let manifest_path = self.manifest_path.display().to_string();
-            self.path().join("Cargo.toml").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)
         };