Take &mut self instead of self in builders

[frozolotl]

See https://rust-lang-nursery.github.io/api-guidelines/type-safety.html#builders-enable-construction-of-complex-values-c-builder

[IvanUkhov]

Could you please be more specific? Which methods are you referring to?

[frozolotl]

I'm referring to most methods in svg::node::element. It would allow writing for example:

let mut document = Document::new();
if cond {
    document.add(…);
}

[IvanUkhov]

I think it’s a good suggestion. I’ve tagged it as “feedback needed” to see if there are other opinions so that we don’t miss anything. If you have time, please feel free to open a pull request.

[MichaelMauderer]

I just ran into the same issue. So I'm just here to pile on.

I have a wrapper struct that is meant to hold a Document and manipulate it. Since add takes the Document by value, this means that a cumbersome dance with Options or mem::replace is needed to make this work.

Example:

struct SVGRenderer {
    document: Document,
}

impl SVGRenderer {
    pub fn new() -> Self {
        SVGRenderer {
            document: Document::new(),
        }
    }

    fn add_to_doc<T: Node>(&mut self, obj: T) {
        self.document = mem::replace(&mut self.document, Document::new()).add(obj);
    }
}

impl Renderer for SVGRenderer {
    fn draw_point(&mut self, p: &Point) {
        let circle = Circle::new()
            .set("cx", p.pos.x)
            .set("cy", p.pos.y)
            .set("r", p.radius)
            .set("stroke", "black")
            .set("stroke-width", 1);
        // Can't do self.document.add(circle)
        // Neither self.document = self.document.add(circle)
        // Needs to be something like this
        self.document = mem::replace(&mut self.document, Document::new()).add(obj);
    }
} 

[IvanUkhov]

Let us continue this discussion in #23. I need your input. Thanks!

[IvanUkhov]

@MichaelMauderer, by the way, in your example, would it not work with append?

[IvanUkhov]

@frozolotl, can’t append help you too?

[MichaelMauderer]

Actually, it does! I had completly missed that function. Thanks for pointing it out.

[IvanUkhov]

I park it for now, as add and set are accompanied by append and assign, respectively.