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

Updating comment #123

Closed
wants to merge 1 commit into from
Closed

Updating comment #123

wants to merge 1 commit into from

Conversation

unional
Copy link

@unional unional commented Aug 11, 2022

While self: Bar also works, but it is not typical and has different behavior. It is better to use self: *Bar

While `self: Bar` also works, but it is not typical and has different behavior. It is better to use `self: *Bar`
@ratfactor
Copy link
Owner

@unional You may be right, but that seems to disagree with this previous Ziglings issue: #89

Here's an example from the Zig docs:

    pub fn dot(self: Vec3, other: Vec3) f32 {
        return self.x * other.x + self.y * other.y + self.z * other.z;
    }

I'm pretty sure a printMe() method would be more correct without the reference since it does not modify the original value?

@unional
Copy link
Author

unional commented Aug 11, 2022

I see. That sounds right. I'm still learning zig and this seems to be zig specific.

I create this PR because the comment and the code in this exercise does not match.

Maybe another way is to create one more exercise to demonstrate this, and update the comment section in this exercise to a mutable example, so that the comment and code are consistent. :)

@ratfactor
Copy link
Owner

@unional Oh, I see what you mean! You're right, the comment and code don't match here. I think I'll make a new issue to re-work the comment on this one. There may be other places where this is no longer consistent. Thanks!

@unional
Copy link
Author

unional commented Aug 19, 2022

Sounds good. thx!

@unional unional closed this Aug 19, 2022
@unional
Copy link
Author

unional commented Aug 19, 2022

btw I did some streaming on going through your exercises: https://www.youtube.com/watch?v=ORR10_c7IKM&list=PLTLdF9P_MLntirxSOWhJoMAiitnCGbKzw

Just to share. Thanks for creating them. 😉

@chrboesch
Copy link
Collaborator

Very nice. Maybe you should add a note that you are giving a spoiler with your video. 😉

@unional
Copy link
Author

unional commented Aug 20, 2022

giving a spoiler with your video.

Haha, sure will do. 🌷

@unional unional deleted the patch-2 branch August 20, 2022 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants