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

ch13-01 use of value as method and field name #1740

Closed
davidnx opened this issue Jan 3, 2019 · 10 comments
Closed

ch13-01 use of value as method and field name #1740

davidnx opened this issue Jan 3, 2019 · 10 comments
Milestone

Comments

@davidnx
Copy link

davidnx commented Jan 3, 2019

In listing 13-10, value is used as a field in struct Cacher<T> but value is also used as a method name in impl<T> Cacher<T>.

An explanation of why this is acceptable, and when it is or isn't a good idea, would be helpful.

Listing 13-9:

struct Cacher<T>
    where T: Fn(u32) -> u32
{
    calculation: T,
    value: Option<u32>, // DEFINED HERE
}

Listing 13-10:

impl<T> Cacher<T>
    where T: Fn(u32) -> u32
{
    fn new(calculation: T) -> Cacher<T> {
        Cacher {
            calculation,
            value: None,
        }
    }

    fn value(&mut self, arg: u32) -> u32 { // DEFINED HERE
        match self.value {
            Some(v) => v,
            None => {
                let v = (self.calculation)(arg);
                self.value = Some(v);
                v
            },
        }
    }
}
@carols10cents
Copy link
Member

There's a spot in Chapter 9 where we talk about this in a similar scenario, search for "getter" on this page: https://doc.rust-lang.org/stable/book/ch09-03-to-panic-or-not-to-panic.html#guidelines-for-error-handling

We also talk about this in Chapter 17 when we discuss encapsulation: https://doc.rust-lang.org/stable/book/ch17-01-what-is-oo.html#encapsulation-that-hides-implementation-details

I feel like we'd be getting a little repetitive if we talked about accessor methods here too, what do you think?

@davidnx
Copy link
Author

davidnx commented Jan 4, 2019

Thanks for the links. In many languages, methods and fields cannot use the same name. The fact that this is possible and unambiguous in Rust is not immediately obvious, and I think readers would benefit from a concise explanation of the resolution rules.

The 3 places you mentioned in the book which implement getter's do not mention how Rust resolves the apparent ambiguity: in the getter method, both the method and the field are in scope, yet the access of the field is unambiguous. Here's a silly snippet that I think exemplifies the behavior that I was unaware of:

pub struct MyStruct {
    pub value: i32,
}

impl MyStruct {
    pub fn new(value: i32) -> MyStruct {
        MyStruct { value }
    }
    
    pub fn value(&self) -> i32 {
        self.value * 2 // Return a different value than the field to illustrate that this was called instead
    }
}

fn main() {
    let a = MyStruct::new(123);
    println!("a.value:   {}", a.value);   // REFERS TO THE FIELD, prints 123
    println!("a.value(): {}", a.value()); // REFERS TO THE METHOD, prints 246
}

@steveklabnik
Copy link
Member

So, I am a bit torn. Depending on what language you're coming from, this is important, but I think it really depends in that regard. I can see both clarifying this, and thinking that maybe this is a bit too niche to really cover. Hrm.

Regardless, thank you for filing!

@davidnx
Copy link
Author

davidnx commented Jan 8, 2019

Which mainstream languages other than Rust also do this? For anyone coming from C, C++, C#, Java, Python, JavaScript, TypeScript, this is new (and surprising) behavior. That's at least 7 of the 10 most popular languages per Github

@steveklabnik
Copy link
Member

At least Ruby. Doing some research and getting some numbers would be good here, though! I've written a bunch of code in those languages and never noticed that this is true; I thought it was for at least some of them.

@eddyp
Copy link
Contributor

eddyp commented Jan 8, 2019

Isn't there a scopes (definitely lifetimes related chapter/info would be counter intuitive, imho) chapter where this could be mentioned explicitly?

@steveklabnik
Copy link
Member

steveklabnik commented Jan 8, 2019

Sorta; chapter 4's ownership section, maybe. But that's not this kind of scope.

@AndreasDavour
Copy link

I just read this part of the book, and was seriously confused. The invocation of the closure was explained with:

"If self.value is None, the code calls the closure stored in self.calculation, saves the result in self.value for future use"

which explains nothing about why

let v = (self.calculation)(arg);

suddenly contains a set of parentheses not ever used in method invocation before in the book!

I got on the ##rust channel on freenode and some helpful rustaceans helped me understand it, but it could be a lot clearer why they are there!

Using the same name of things necessitates you using those parentheses, but as this chapter is all about something I would consider a bit challenging, closures not being found in classic languages like C or Fortran, I wonder if that extra density doesn't distract from the core idea being taught in the chapter? I for one was really thrown for a loop!

Reading what has been written in this issue above, I can see the point for why you would use the same name for both a method and a field name. But, it caused a lot of confusion in this case, where there were already a lot to take in.

By the way, that something is mentioned in chapter 17 argues strongly for additional explanation in this chapter. The first chapter explicitly say the books is written to be read front to back!

Also, using parentheses like this made me totally miss the (arg) bit was just regular function call parameters wrapped in brackets. To me it looked like two odd expressions in brackets after each other. I'd really like to at least have the last paragraph after Listing 13-10 mention why the self.calculation expression looks like it does.

Referring back to chapter 9 and what was happening back there probably doesn't hurt either.

@carols10cents
Copy link
Member

I'm going to put this as chapter 5 to start; I think when we discuss structs and methods we can add a note that a struct field is different from a method. When that's fixed, I'll move this to milestones for any later chapters we decide also need revision to address this issue.

@carols10cents carols10cents added this to the ch5 milestone Jul 17, 2021
@carols10cents carols10cents modified the milestones: ch5, ch13 Aug 1, 2021
carols10cents added a commit that referenced this issue Aug 2, 2021
@carols10cents
Copy link
Member

I've decided to rework the closure examples, and I won't repeat the same issue with value as a method and field name that prompted this issue. So the note added to chapter 5 in b4132ae now closes this issue.

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

5 participants