rsm: Split First into two options, and generate Set earlier during parsing.

Emmanuel Gil Peyrot created 8 years ago

Change summary

src/rsm.rs | 83 ++++++++++++++++++++++---------------------------------
1 file changed, 34 insertions(+), 49 deletions(-)

Detailed changes

src/rsm.rs

@@ -12,18 +12,13 @@ use error::Error;
 
 use ns;
 
-#[derive(Debug, Clone)]
-pub struct First {
-    pub index: Option<usize>,
-    pub id: String,
-}
-
 #[derive(Debug, Clone)]
 pub struct Set {
     pub after: Option<String>,
     pub before: Option<String>,
     pub count: Option<usize>,
-    pub first: Option<First>,
+    pub first: Option<String>,
+    pub first_index: Option<usize>,
     pub index: Option<usize>,
     pub last: Option<String>,
     pub max: Option<usize>,
@@ -36,68 +31,61 @@ impl<'a> TryFrom<&'a Element> for Set {
         if !elem.is("set", ns::RSM) {
             return Err(Error::ParseError("This is not a RSM element."));
         }
-        let mut after = None;
-        let mut before = None;
-        let mut count = None;
-        let mut first = None;
-        let mut index = None;
-        let mut last = None;
-        let mut max = None;
+        let mut set = Set {
+            after: None,
+            before: None,
+            count: None,
+            first: None,
+            first_index: None,
+            index: None,
+            last: None,
+            max: None,
+        };
         for child in elem.children() {
             if child.is("after", ns::RSM) {
-                if after.is_some() {
+                if set.after.is_some() {
                     return Err(Error::ParseError("Set can’t have more than one after."));
                 }
-                after = Some(child.text());
+                set.after = Some(child.text());
             } else if child.is("before", ns::RSM) {
-                if before.is_some() {
+                if set.before.is_some() {
                     return Err(Error::ParseError("Set can’t have more than one before."));
                 }
-                before = Some(child.text());
+                set.before = Some(child.text());
             } else if child.is("count", ns::RSM) {
-                if count.is_some() {
+                if set.count.is_some() {
                     return Err(Error::ParseError("Set can’t have more than one count."));
                 }
-                count = Some(child.text().parse()?);
+                set.count = Some(child.text().parse()?);
             } else if child.is("first", ns::RSM) {
-                if first.is_some() {
+                if set.first.is_some() {
                     return Err(Error::ParseError("Set can’t have more than one first."));
                 }
-                first = Some(First {
-                    index: match child.attr("index") {
-                        Some(index) => Some(index.parse()?),
-                        None => None,
-                    },
-                    id: child.text(),
-                });
+                set.first_index = match child.attr("index") {
+                    Some(index) => Some(index.parse()?),
+                    None => None,
+                };
+                set.first = Some(child.text());
             } else if child.is("index", ns::RSM) {
-                if index.is_some() {
+                if set.index.is_some() {
                     return Err(Error::ParseError("Set can’t have more than one index."));
                 }
-                index = Some(child.text().parse()?);
+                set.index = Some(child.text().parse()?);
             } else if child.is("last", ns::RSM) {
-                if last.is_some() {
+                if set.last.is_some() {
                     return Err(Error::ParseError("Set can’t have more than one last."));
                 }
-                last = Some(child.text());
+                set.last = Some(child.text());
             } else if child.is("max", ns::RSM) {
-                if max.is_some() {
+                if set.max.is_some() {
                     return Err(Error::ParseError("Set can’t have more than one max."));
                 }
-                max = Some(child.text().parse()?);
+                set.max = Some(child.text().parse()?);
             } else {
                 return Err(Error::ParseError("Unknown child in set element."));
             }
         }
-        Ok(Set {
-            after: after,
-            before: before,
-            count: count,
-            first: first,
-            index: index,
-            last: last,
-            max: max,
-        })
+        Ok(set)
     }
 }
 
@@ -116,14 +104,10 @@ impl<'a> Into<Element> for &'a Set {
             elem.append_child(Element::builder("count").ns(ns::RSM).append(format!("{}", self.count.unwrap())).build());
         }
         if self.first.is_some() {
-            let first = self.first.clone().unwrap();
             elem.append_child(Element::builder("first")
                                       .ns(ns::RSM)
-                                      .attr("index", match first.index {
-                                           Some(index) => Some(format!("{}", index)),
-                                           None => None,
-                                       })
-                                      .append(first.id.clone()).build());
+                                      .attr("index", self.first_index.map(|index| format!("{}", index)))
+                                      .append(self.first.clone()).build());
         }
         if self.index.is_some() {
             elem.append_child(Element::builder("index").ns(ns::RSM).append(format!("{}", self.index.unwrap())).build());
@@ -188,6 +172,7 @@ mod tests {
             before: None,
             count: None,
             first: None,
+            first_index: None,
             index: None,
             last: None,
             max: None,