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 10e0e3d2a466196e406485e17d9b466a1b8e360e..0000000000000000000000000000000000000000 --- 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 58acc309198fc9a2313740b71dbd0fe411f090e6..4d3d59e1f10a1c9e949e6885eff166c78366c39b 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 4728608db797f5ee40e62273acd5bffa58e38f8e..23e234c93f4205249f305a26e331a7dc9b65bd9e 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 64a32f920d49db61214b3522f7976d2b1eaf4546..f58bb6ebb1804f095bb928ece8fb459da3dc6029 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")