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

arc1 seems like a giant step #250

Closed
chrisridd opened this issue Dec 18, 2019 · 18 comments · Fixed by #710
Closed

arc1 seems like a giant step #250

chrisridd opened this issue Dec 18, 2019 · 18 comments · Fixed by #710

Comments

@chrisridd
Copy link

There are several new concepts in the arc1.rs exercise, and I wonder if it would be useful to have a few gentle lead ins before this exercise.

In particular threads are a big leap in complexity with some new syntax (closures).

It would also be helpful to know what the arc1 code is trying to do in the first place. Reverse engineering the logic (with known missing lines of code) and figuring out threads and figuring out Arc, seems like too much for one exercise.

@AbdouSeck
Copy link
Contributor

Hi @chrisridd, I agree that Arc requires some familiarity with both threads and closures, but since we already have an exercise on threads, it does kind of make sense to assume that anyone who's completed the threads exercise should be, in some way, familiar with how they are spawned (via closures).
What I recommend is adding some language (documentation) indicating threads are spawned using closures; or maybe moving the arc1.rs file to the threads folder?
Let me know what you think.

Thank you,

Abdou

@chrisridd
Copy link
Author

Hi @AbdouSeck, the problem at the moment is the threads exercises come after arc1.rs. So it sounds like moving arc1.rs to the threads folder would be a good solution!

@AbdouSeck
Copy link
Contributor

Awesome! Go for it!

@sl4m
Copy link
Contributor

sl4m commented Mar 7, 2020

Hi @AbdouSeck, the problem at the moment is the threads exercises come after arc1.rs. So it sounds like moving arc1.rs to the threads folder would be a good solution!

Just an FYI, a previous issue (#205) suggested the opposite and this commit fbe91a6 was made.

But I agree with what @chrisridd. It was a big leap. I think it might be helpful for the README under standard_library_types to simply say to read Chapter 16 in its entirety. This helped me understand thread::spawn usage and its return type. I still had trouble understanding what the exercise wanted me to do. It was not clear there was a relationship with shared_numbers and child_numbers. One could easily "solve" the problem by initialising the child_numbers with a completely new Vec, but that's not the intent of the exercise.

@jhscheer
Copy link

It would also be helpful to know what the arc1 code is trying to do in the first place. Reverse engineering the logic (with known missing lines of code) and figuring out threads and figuring out Arc, seems like too much for one exercise.

I agree!
Arc1 seems way too hard for the wrong reasons.
At a minimum the introduction should tell the reader what this program's goal is.
Even better would be an example println output.

@steelx
Copy link

steelx commented May 3, 2020

Im stuck at Arc1. Threads exercise never showed up.. how to `go for it :)

in-case anyone is stuck like me please read : https://medium.com/@DylanKerler1/how-arc-works-in-rust-b06192acd0a6

let shared_numbers will need be an Arc
and child_numbers will read the Arc reference

@MrEmanuel
Copy link

MrEmanuel commented May 11, 2020

I too had problems with Arc1.
I know about threads and closures, so that's not a big problem. Although, I'm not sure what the purpose of the exercise is.
I'm creating a variable shared_numbers by passing numbers to Arc::new(). Then I make a copy of shared_numbers on each iteration in the main thread. It compiles and prints offset values but I have no idea why or what I'm actually suppose to learn.
Why does cloning shared_numbers to the child_numbers variable on each iteration matter?

It seems very strange to me that you

  1. Create a vector of values
  2. Wrap them in an Arc.
  3. Copy that Arc on each iteration.

Why couldn't you just do all those three steps once on each iteration?
Like so; let child_numbers: Arc<Vec<_>> = Arc::new((0..100u32).collect());
It seems to work..

I think I did what I was suppose to, but I have no idea why. So, not a great learning experience. :P
I would make a PR if I knew what I'm suppose to do and how to fix it.

@jhiver
Copy link

jhiver commented Sep 27, 2020

+1, can we maybe have some tests to figure out what the output should be. I can't figure out what this exercice wants me to do.

@a2800276
Copy link

I agree that this exercise is completely out of place. Consider that the next (iterator) exercise basically asks you to figure out which string comes after "banana" in the list.

Presumably most people can just read the hint and bang on the keyboard until it works like I did. But I don't feel Iearned anything.

Also: atomic reference counting seems to be something specific to Rust and (in my limited experience) in C++ where it's a rather esoteric concept. Is Arc really so commonly used that this should be the second exercise in standard_library_types even before iterators?

@ensooboka
Copy link

+1 but I'm also an idiot.

@doup
Copy link

doup commented Dec 30, 2020

I came to open an issue on arc1… and here I am. I'm not the only one that felt the same.

Should this be moved for later? Maybe threads and closures can be explained first?

Whatever, back to reading whole Chapter 16 as sl4m suggests.

@ghost
Copy link

ghost commented Mar 6, 2021

I second @doup

@a2800276
Copy link

a2800276 commented Mar 8, 2021

I see a bit of a dilemma in that the "learners" agreeing that this material is too advanced in this position don't feel qualified to judge where this should be moved too and the authors of the advanced material have moved on/don't agree/don't care. This issue has been around for ages. Does anyone subscribing to this issue know if there is a project lead that could be pinged or is rustlings just a free-for-all?

@shadows-withal
Copy link
Member

@a2800276 This issue has been around for ages, correct, and I agree that something should be done about it. However, there's a couple of things to note about this exercise and this project (which should hopefully answer your question):

  1. Most of these exercises are pretty old. They were written back in 2016 and before, and really only got a big update with the 2018 edition. Ideally, there should be a complete makeover that takes into account the exercise order and adds/removes exercises to adjust more to how people learn Rust nowadays.
  2. Antithetical to the above, despite this repository being the second most-starred one in the rust-lang org, there's only 3 volunteer maintainers for it, who aren't even properly represented in the Rust project team structure (this is mostly our negligence, though). While we do occasionally get drive-by pull requests for fixes and rarely even features, the fact remains that those pull requests still need to be reviewed. All of us maintainers have a job or other things to do, and motivation is always fluctuating. In my experience, the motivation (at least when you're doing this in your free time) gets less and less the higher-profile the project you're maintaining is, for one reason or another.

For arc1 to be reworked in a significant manner, three things could happen:

  1. A kind stranger/contributor does it out of pure goodwill and altruism.
  2. One of us maintainers finds the time and motivation to do it.
  3. Someone pays us or one of us to do it.

None of these are very likely to happen (although maybe now with the Rust Foundation it might be easier to get some funding for keeping this project up to date), which is why this issue and some others are in the state they're in. Hope this helps!

@a2800276
Copy link

@fmoko

Of course I realize that the maintainers are volunteers. My whole point was the paradox that by the nature of this project, the actual users are not qualified to provide fixes. This is especially worrisome considering rustlings is the 2nd popular rust repo paired with your statement that the course needs a makeover to reflect contemporary usage.

Maybe at least a warning could be added to the repo, the learners using it can't be expected to judge the relevance of exercises, because rustlings is likely their first contact with the language. I can only speak for myself, but this exercise made me give up rustlings and focus on learning by book.

@shadows-withal
Copy link
Member

shadows-withal commented Mar 21, 2021

Hm, I understand your concern, but I'm not sure whether doing something as drastic as that is useful to the project and the state of learning Rust. I wouldn't say that rustlings is not relevant to the language in its current state, the "core" features (which is most of what we're teaching here) basically don't change at all. It's really only exercises like this that document fairly advanced features of the language/standard library that can become outdated and in need of a refactor.

arc1 and the other advanced exercises were basically an experiment to see if we can teach more advanced concepts in the same manner we taught simple ones. If we were to do a thorough review and figure out that, no, this doesn't work the same way, we'd probably remove them! Rustlings has a scope that mostly focuses on the more elementary concepts, and it's possible that we might be able to trim the amount of exercises to provide a more concise curriculum.

An aside, I don't think that "users are not qualified to provide fixes" at all. There have been plenty of good ideas and starting points in this issue and others, but this goes back to my previous comment: Someone needs to take the initiative. Sometimes this happens, sometimes this doesn't, that's how open source works.

@bmacer
Copy link
Contributor

bmacer commented Apr 20, 2021

Greetings,

I created a pull request (#710) that will hopefully alleviate this issue and perhaps close it. I read through the issue and though I'm new to rust, it seemed an opportunity to contribute. Rustlings is an amazing little tool. :-). I was also struck by @fmoko's comments that they would "probably remove [these advanced concepts]" which makes me sad because I think there's a place for more advanced concepts in this wonderful project. I was also struck by @fmoko's disagreement that "users are not qualified to provide fixes" and took it as a personal challenge. I've never contributed to a Github in any substantial (or semi-substantial) manner and thought "hey, maybe I can learn something and help out too. " So I read Chapter 16 (per this issue's thread), figured out what the point of this question is, and tried to color the intro comments to make it more clear to others too.

The only actual code change I made was adding let child_numbers = // TODO to the place where it ultimately belongs (previously it was a blank line). Hopefully having the two TODOs clearly labeled will make it more likely someone might be able to stumble their way into the answer (always a good feeling).

Hopefully my pull request is helpful to the community.

@omdxp
Copy link

omdxp commented Jul 20, 2022

fn main() {
    let numbers: Vec<_> = (0..100u32).collect();
    let shared_numbers = Arc::new(numbers); // TODO
    let mut joinhandles = Vec::new();

    for offset in 0..8 {
        let child_numbers = Arc::clone(&shared_numbers); // TODO
        joinhandles.push(thread::spawn(move || {
            let sum: u32 = child_numbers.iter().filter(|n| *n % 8 == offset).sum();
            println!("Sum of offset {} is {}", offset, sum);
        }));
    }
    for handle in joinhandles.into_iter() {
        handle.join().unwrap();
    }
}

I'm new to using Arc, I have followed the documentation, but it's still vague for me

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 a pull request may close this issue.