Merge pull request #818 from rnestler/libimagdiary/refactor_from_store_id

[WIP] Use Result for from_storeid

libimagdiary/src/diaryid.rs

@@ -30,6 +30,11 @@ use libimagstore::storeid::StoreId;
 use libimagstore::storeid::IntoStoreId;
 use libimagstore::store::Result as StoreResult;
 
+use error::DiaryError as DE;
+use error::DiaryErrorKind as DEK;
+use error::MapErrInto;
+use libimagerror::into::IntoError;
+
 use module_path::ModuleEntryPath;
 
 #[derive(Debug, Clone)]
@@ -175,27 +180,37 @@ impl Into<NaiveDateTime> for DiaryId {
 
 pub trait FromStoreId : Sized {
 
-    fn from_storeid(&StoreId) -> Option<Self>;
+    fn from_storeid(&StoreId) -> Result<Self, DE>;
 
 }
 
 use std::path::Component;
 
-fn component_to_str<'a>(com: Component<'a>) -> Option<&'a str> {
+fn component_to_str<'a>(com: Component<'a>) -> Result<&'a str, DE> {
     match com {
         Component::Normal(s) => Some(s),
-        _ => None
+        _ => None,
     }.and_then(|s| s.to_str())
+    .ok_or(DEK::IdParseError.into_error())
 }
 
 impl FromStoreId for DiaryId {
 
-    fn from_storeid(s: &StoreId) -> Option<DiaryId> {
+    fn from_storeid(s: &StoreId) -> Result<DiaryId, DE> {
         use std::str::FromStr;
 
+        use std::path::Components;
+        use std::iter::Rev;
+
+        fn next_component<'a>(components: &'a mut Rev<Components>) -> Result<&'a str, DE> {
+            components.next()
+                .ok_or(DEK::IdParseError.into_error())
+                .and_then(component_to_str)
+        }
+
         let mut cmps   = s.components().rev();
-        let (hour, minute) = match cmps.next().and_then(component_to_str)
+
-            .and_then(|time| {
+        let (hour, minute) = try!(next_component(&mut cmps).and_then(|time| {
             let mut time = time.split(":");
             let hour     = time.next().and_then(|s| FromStr::from_str(s).ok());
             let minute   = time.next()
@@ -206,31 +221,36 @@ impl FromStoreId for DiaryId {
             debug!("Minute = {:?}", minute);
 
             match (hour, minute) {
-                    (Some(h), Some(m)) => Some((h, m)),
+                (Some(h), Some(m)) => Ok((h, m)),
-                    _ => None,
+                _ => return Err(DE::new(DEK::IdParseError, None)),
             }
-            })
+        }));
-        {
-            Some(s) => s,
-            None => return None,
-        };
 
-        let day   :Option<u32> = cmps.next().and_then(component_to_str).and_then(|s| FromStr::from_str(s).ok());
+        let day: Result<u32,_> = next_component(&mut cmps)
-        let month :Option<u32> = cmps.next().and_then(component_to_str).and_then(|s| FromStr::from_str(s).ok());
+            .and_then(|s| s.parse::<u32>()
-        let year  :Option<i32> = cmps.next().and_then(component_to_str).and_then(|s| FromStr::from_str(s).ok());
+                      .map_err_into(DEK::IdParseError));
-        let name       = cmps.next().and_then(component_to_str).map(String::from);
+
+        let month: Result<u32,_> = next_component(&mut cmps)
+            .and_then(|s| s.parse::<u32>()
+                      .map_err_into(DEK::IdParseError));
+
+        let year: Result<i32,_> = next_component(&mut cmps)
+            .and_then(|s| s.parse::<i32>()
+                      .map_err_into(DEK::IdParseError));
+
+        let name = next_component(&mut cmps).map(String::from);
 
         debug!("Day   = {:?}", day);
         debug!("Month = {:?}", month);
         debug!("Year  = {:?}", year);
         debug!("Name  = {:?}", name);
 
-        let day    = match day   { None => return None, Some(day)   => day };
+        let day    = try!(day);
-        let month  = match month { None => return None, Some(month) => month };
+        let month  = try!(month);
-        let year   = match year  { None => return None, Some(year)  => year };
+        let year   = try!(year);
-        let name   = match name  { None => return None, Some(name)  => name };
+        let name   = try!(name);
 
-        Some(DiaryId::new(name, year, month, day, hour, minute))
+        Ok(DiaryId::new(name, year, month, day, hour, minute))
     }
 
 }

libimagdiary/src/error.rs

@@ -27,7 +27,8 @@ generate_error_module!(
         PathConversionError => "Error while converting paths internally",
         EntryNotInDiary     => "Entry not in Diary",
         IOError             => "IO Error",
-        ViewError           => "Error viewing diary entry"
+        ViewError           => "Error viewing diary entry",
+        IdParseError        => "Error while parsing ID"
     );
 );
 

libimagdiary/src/iter.rs

@@ -30,6 +30,7 @@ use entry::Entry as DiaryEntry;
 use error::DiaryError as DE;
 use error::DiaryErrorKind as DEK;
 use result::Result;
+use libimagerror::trace::trace_error;
 
 /// A iterator for iterating over diary entries
 pub struct DiaryEntryIterator<'a> {
@@ -99,9 +100,10 @@ impl<'a> Iterator for DiaryEntryIterator<'a> {
             if next.is_in_diary(self.name) {
                 debug!("Seems to be in diary: {:?}", next);
                 let id = match DiaryId::from_storeid(&next) {
-                    Some(i) => i,
+                    Ok(i) => i,
-                    None => {
+                    Err(e) => {
-                        debug!("Couldn't parse {:?} into DiaryId", next);
+                        trace_error(&e);
+                        debug!("Couldn't parse {:?} into DiaryId: {:?}", next, e);
                         continue;
                     }
                 };