Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some methods that change state use &self instead of &mut self #116

Open
nstoddard opened this issue Jan 30, 2015 · 4 comments
Open

Some methods that change state use &self instead of &mut self #116

nstoddard opened this issue Jan 30, 2015 · 4 comments

Comments

@nstoddard
Copy link

In commit bba330b, several methods that took &mut self were changed to take &self instead. Why was this change made? It seems like a bad idea to me. Several of those methods change internal state, so omitting mut might cause problems.

@andrewrk
Copy link
Member

Mutability is referring to whether any of the fields of face need to change when that method is called. Although some internal state might change in the freetype library code, the Face struct itself does not need to change.

@nstoddard
Copy link
Author

Some projects such as glfw-rs use &mut self even for methods that don't modify struct fields. See PistonDevelopers/glfw-rs@6b5c2fe for an example. It seems like we should follow that example. For one thing, it's conceivable that at some point we might want to change some of those methods so that they do change fields, which would require switching to &mut self anyway. I'd rather not risk having to break the API like that.

@bvssvni
Copy link
Member

bvssvni commented Feb 2, 2015

I asked in #rust-internals about this, and was told the calling order of FFI functions won't be affected.

@bvssvni
Copy link
Member

bvssvni commented Feb 2, 2015

This seems like an issue similar to when to use unsafe, which also has grey areas in library design. I can't tell which way is right, because there could be different contexts where either case is wrong. The argument in favor of using &mut self is that the API breaks less code when doing &mut self => &self, than &self => &mut self. If we suspect we will run into this issue and there is no context we can think of when &self is better, then I think we should use &mut self.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants