Files: Sometimes, clicking "back" or "forward" during the load of a large directory will include an item from the previous folder in the new one
Large directories are notorious in this. I currently have a cancellation token that triggers when navigation commands are invoked, but it still includes one random item from the previous directory in the directory being navigated to.
duke7553
All 5 comments
You can fix the problem quickly by moving the cancellation token check closer to the list addition code. If things get crazy you can use a lock for accessing the token but I don't think that will be necessary.
Imo it's a problem with the architecture though. The issue exists because you are trying to add to the same list even though you are creating a new ItemViewModel. Almost all of the fields are static, but you have a singleton field as well (ItemViewModel.ViewModel). Using the singleton at least will solve this issue. To me both are evil and you should be using plain old instances when possible, i.e. some class should just be holding an instance of ItemViewModel and this instance should be called by everyone instead of using static or singleton.
This is very much a opinion thing though and each programmer has his own tastes developed over years. Just because I say something doesn't make it right (plus I only have 5 years exp and not in C# exclusively). I remember from reddit that you are in high school. You have a popular OSS project people are interested in and are contributing to, use this opportunity to learn good coding practices 馃憤
ashikns
on 19 Feb 2019
@duke7553 Thank you. It really makes me proud to hear all of this feedback. I have a solution which you can expect to come very soon for the "architecture problem". I followed your advice above so Files won't create an entirely new ItemViewModel every single time.
duke7553
on 20 Feb 2019
@duke7553 I created a new PR containing the changes. It's a small step in the right direction, but it still counts.
duke7553
on 20 Feb 2019
@duke7553 Is this issue solved? I couldn't reproduce in the latest master - ##256a881ef1df035bdcd6898931f759c3125e262e.
ashikns
on 5 Mar 2019
This should be fixed now
duke7553
on 6 Mar 2019
Related issues
SinGuLaRiTy2001
路
3Comments
Francisco820
路
3Comments
yaichenbaum
路
3Comments
generalguy41
路
3Comments
Dylan-Osborne
路
3Comments
Most helpful comment
You can fix the problem quickly by moving the cancellation token check closer to the list addition code. If things get crazy you can use a lock for accessing the token but I don't think that will be necessary.
Imo it's a problem with the architecture though. The issue exists because you are trying to add to the same list even though you are creating a new ItemViewModel. Almost all of the fields are static, but you have a singleton field as well (ItemViewModel.ViewModel). Using the singleton at least will solve this issue. To me both are evil and you should be using plain old instances when possible, i.e. some class should just be holding an instance of ItemViewModel and this instance should be called by everyone instead of using static or singleton.
This is very much a opinion thing though and each programmer has his own tastes developed over years. Just because I say something doesn't make it right (plus I only have 5 years exp and not in C# exclusively). I remember from reddit that you are in high school. You have a popular OSS project people are interested in and are contributing to, use this opportunity to learn good coding practices 馃憤