Skip to content

Pass slices to functions in exercise 9.6.1 (#2848) #2850

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

Closed
wants to merge 1 commit into from

Conversation

iurii-kyrylenko
Copy link

@iurii-kyrylenko iurii-kyrylenko commented Aug 16, 2025

Fixes issue #2848:
Use the references to a Dynamically Sized Slice instead of references to a Fixed-Size Array in function parameters.

Copy link

google-cla bot commented Aug 16, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

djmitche
djmitche previously approved these changes Aug 16, 2025
Copy link
Collaborator

@djmitche djmitche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@mgeisler
Copy link
Contributor

Have people learned about slices at this point?

@mgeisler
Copy link
Contributor

Have people learned about slices at this point?

Ah, yes, I see they have 😄 Slices are introduced in 9.3 Slices.

However, the exercise text says:

utility functions for 3-dimensional geometry, representing a point as [f64; 3].

So that is the reason why the functions use the [f64; 3] type — it's a simple concrete type which people should have an easy time understanding.

When I've been teaching the class (both inside and outside of Google), I'm sometimes met with questions about the math-related exercises. That is, while magnitude works for higher dimensions, the math is not the point of this exercise. The use of shared and unique references is the point.

I would suggest keeping the solution as-is, but add a speaker note showing that one could use slices instead. The speaker note should then discuss the tradeoffs: with slices, you have a runtime length check, but with borrowed arrays, you have compile-time knowledge of the length (and the compiler can unroll the loops accordingly).

Basically, I don't think it's 100% clear that always using slices is the right call. A vector-library would want to use const generics instead, I believe.

@djmitche
Copy link
Collaborator

That's a good point, @mgeisler -- thank you! I had missed that the instructions specifically mention arrays.

This solution already has a speaker note, so adding another regarding slices seems like a good idea (and this is the segment to do it in, since we have discussed both references and slices).

@djmitche djmitche dismissed their stale review August 17, 2025 02:13

(agreeing with @mgeisler)

@iurii-kyrylenko
Copy link
Author

I would suggest keeping the solution as-is, but add a speaker note showing that one could use slices instead. The speaker note should then discuss the tradeoffs.

@djmitche, Can we ask @mgeisler to add a speaker note?

We could mention that declaring slice &[T] as function params has a flexibility, based on deref coercions:

  • if we have a slice &[T], we can pass that directly;
  • if we have an array [T; N], we can pass a slice &[T] or a reference to the array &[T;N].

The same holds for string slices as parameters.

@djmitche
Copy link
Collaborator

Fair point - I made #2851 for that purpose.

The Deref trait is covered (indirectly) later in the course, so I think it's best to omit deref coercion here.

@djmitche djmitche closed this Aug 17, 2025
@iurii-kyrylenko iurii-kyrylenko deleted the fix-2848 branch August 17, 2025 20:02
@iurii-kyrylenko
Copy link
Author

Thank you, @djmitche !

@mgeisler
Copy link
Contributor

@djmitche, Can we ask @mgeisler to add a speaker note?

Yes, certainly! I've been meaning to get back into the habit of contributing PRs! 🙂

Thanks for the PR, both of you!

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.

Solution for exercise 9.6.1: Use references to a Dynamically Sized Slice instead of references to a Fixed-Size Array.
3 participants