From f87e2442d4350d187ce5403bb87919437cdfab21 Mon Sep 17 00:00:00 2001 From: Bastien Orivel Date: Wed, 26 Apr 2017 23:44:58 +0200 Subject: [PATCH] Use a BTreeMap instead of a Vec to store attributes This way we don't need to reimplement PartialEq for Element. It's also way easier to get an attribute by name as we don't need to iterate over every attribute to see if it exists. The only side effect is that now, in the Debug output, attributes are automatically sorted by names instead of being sorted by insertion order. Fixes #4 --- src/attribute.rs | 45 ----------------------------- src/element.rs | 75 ++++++++++++++++++------------------------------ src/lib.rs | 2 -- src/tests.rs | 2 +- 4 files changed, 29 insertions(+), 95 deletions(-) delete mode 100644 src/attribute.rs diff --git a/src/attribute.rs b/src/attribute.rs deleted file mode 100644 index 10e0e3d2..00000000 --- a/src/attribute.rs +++ /dev/null @@ -1,45 +0,0 @@ -//! Provides an `Attribute` type which represents an attribute in an XML document. - -use xml::escape::escape_str_attribute; - -use std::fmt; - -/// An attribute of a DOM element. -/// -/// This is of the form: `name`="`value`" -/// -/// This does not support prefixed/namespaced attributes yet. -#[derive(Clone, Debug, PartialEq, Eq, Ord, PartialOrd)] -pub struct Attribute { - /// The name of the attribute. - pub name: String, - /// The value of the attribute. - pub value: String, -} - -impl fmt::Display for Attribute { - fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - write!(fmt, "{}=\"{}\"", self.name, escape_str_attribute(&self.value)) - } -} - -impl Attribute { - /// Construct a new attribute from the given `name` and `value`. - /// - /// # Examples - /// - /// ``` - /// use minidom::Attribute; - /// - /// let attr = Attribute::new("name", "value"); - /// - /// assert_eq!(attr.name, "name"); - /// assert_eq!(attr.value, "value"); - /// ``` - pub fn new, V: Into>(name: N, value: V) -> Attribute { - Attribute { - name: name.into(), - value: value.into(), - } - } -} diff --git a/src/element.rs b/src/element.rs index 58acc309..4d3d59e1 100644 --- a/src/element.rs +++ b/src/element.rs @@ -2,13 +2,13 @@ use std::io::prelude::*; use std::io::Cursor; +use std::collections::BTreeMap; +use std::iter::FromIterator; use std::fmt; use error::Error; -use attribute::Attribute; - use xml::reader::{XmlEvent as ReaderEvent, EventReader}; use xml::writer::{XmlEvent as WriterEvent, EventWriter}; use xml::name::Name; @@ -20,28 +20,15 @@ use std::slice; use convert::{IntoElements, IntoAttributeValue, ElementEmitter}; -#[derive(Clone, Eq)] +#[derive(Clone, PartialEq, Eq)] /// A struct representing a DOM Element. pub struct Element { name: String, namespace: Option, - attributes: Vec, + attributes: BTreeMap, children: Vec, } -impl PartialEq for Element { - fn eq(&self, other: &Element) -> bool { - let mut my_attr = self.attributes.clone(); - my_attr.sort(); - let mut other_attr = other.attributes.clone(); - other_attr.sort(); - - self.name == other.name && - self.namespace == other.namespace && - my_attr == other_attr && - self.children == other.children - } -} impl fmt::Debug for Element { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { @@ -50,7 +37,7 @@ impl fmt::Debug for Element { write!(fmt, " xmlns=\"{}\"", ns)?; } for attr in &self.attributes { - write!(fmt, " {}", attr)?; + write!(fmt, " {}=\"{}\"", attr.0, attr.1)?; } if self.children.is_empty() { write!(fmt, "/>")?; @@ -92,7 +79,7 @@ pub enum Node { } impl Element { - fn new(name: String, namespace: Option, attributes: Vec, children: Vec) -> Element { + fn new(name: String, namespace: Option, attributes: BTreeMap, children: Vec) -> Element { Element { name: name, namespace: namespace, @@ -122,7 +109,7 @@ impl Element { /// ``` pub fn builder>(name: S) -> ElementBuilder { ElementBuilder { - root: Element::new(name.into(), None, Vec::new(), Vec::new()), + root: Element::new(name.into(), None, BTreeMap::new(), Vec::new()), } } @@ -144,7 +131,7 @@ impl Element { Element { name: name.into(), namespace: None, - attributes: Vec::new(), + attributes: BTreeMap::new(), children: Vec::new(), } } @@ -162,10 +149,8 @@ impl Element { /// Returns a reference to the value of the given attribute, if it exists, else `None`. pub fn attr(&self, name: &str) -> Option<&str> { - for attr in &self.attributes { - if attr.name == name { - return Some(&attr.value); - } + if let Some(value) = self.attributes.get(name) { + return Some(&value) } None } @@ -174,14 +159,14 @@ impl Element { pub fn set_attr, V: IntoAttributeValue>(&mut self, name: S, val: V) { let name = name.into(); let val = val.into_attribute_value(); - for attr in &mut self.attributes { - if attr.name == name { - attr.value = val.expect("removing existing value via set_attr, this is not yet supported (TODO)"); // TODO - return; - } + + if let Some(value) = self.attributes.get_mut(&name) { + *value = val.expect("removing existing value via set_attr, this is not yet supported (TODO)"); // TODO + return; } + if let Some(val) = val { - self.attributes.push(Attribute::new(name, val)); + self.attributes.insert(name, val); } } @@ -212,13 +197,11 @@ impl Element { ReaderEvent::StartElement { name, attributes, namespace } => { let attributes = attributes.into_iter() .map(|o| { - Attribute::new( - match o.name.prefix { - Some(prefix) => format!("{}:{}", prefix, o.name.local_name), - None => o.name.local_name - }, - o.value - ) + (match o.name.prefix { + Some(prefix) => format!("{}:{}", prefix, o.name.local_name), + None => o.name.local_name + }, + o.value) }) .collect(); let ns = if let Some(ref prefix) = name.prefix { @@ -247,13 +230,11 @@ impl Element { ReaderEvent::StartElement { name, attributes, namespace } => { let attributes = attributes.into_iter() .map(|o| { - Attribute::new( - match o.name.prefix { - Some(prefix) => format!("{}:{}", prefix, o.name.local_name), - None => o.name.local_name - }, - o.value - ) + (match o.name.prefix { + Some(prefix) => format!("{}:{}", prefix, o.name.local_name), + None => o.name.local_name + }, + o.value) }) .collect(); let ns = if let Some(ref prefix) = name.prefix { @@ -297,7 +278,7 @@ impl Element { start = start.default_ns(ns.as_ref()); } for attr in &self.attributes { // TODO: I think this could be done a lot more efficiently - start = start.attr(Name::local(&attr.name), &attr.value); + start = start.attr(Name::local(&attr.0), &attr.1); } writer.write(start)?; for child in &self.children { @@ -582,7 +563,7 @@ impl ElementBuilder { fn test_element_new() { let elem = Element::new( "name".to_owned() , Some("namespace".to_owned()) - , vec![ Attribute::new("name", "value") ] + , BTreeMap::from_iter(vec![ ("name".to_string(), "value".to_string()) ].into_iter() ) , Vec::new() ); assert_eq!(elem.name(), "name"); diff --git a/src/lib.rs b/src/lib.rs index 4728608d..23e234c9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -67,13 +67,11 @@ extern crate xml; pub mod error; -pub mod attribute; pub mod element; pub mod convert; #[cfg(test)] mod tests; pub use error::Error; -pub use attribute::Attribute; pub use element::{Element, Node, Children, ChildrenMut, ElementBuilder}; pub use convert::{IntoElements, IntoAttributeValue}; diff --git a/src/tests.rs b/src/tests.rs index 64a32f92..f58bb6eb 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -7,7 +7,7 @@ use xml::writer::EventWriter; use element::Element; -const TEST_STRING: &'static str = r#"meownya"#; +const TEST_STRING: &'static str = r#"meownya"#; fn build_test_tree() -> Element { let mut root = Element::builder("root")