On Sun, 8 Aug 2021 11:51:47 +0200 Kristian Amlie <[email protected]> wrote:
>On 08.08.2021 05:35, Rainer Hans Liffers wrote: >> Hi Will, Hermann, and Kristian >> >> On 8/8/21 9:35 am, Ichthyostega wrote: >>> >>> Hi Will, >>> Hi Kristian, >>> >>> today I travelled by train back to Munich, and so I had ample time >>> to look at the indicated changes. Basically I followed all usages >>> of the MasterUI::filerlist and the class FilerLine. >>> >>> While I am rather not familiar with details of FLTK, the usage of >>> the Vector and how you clear it before reading in a new directory >>> or when closing the UI all looks correct for me. >>> >>> >>> At that point, the next question is: how much do we want to go >>> down the rabbit hole? Because, on second thought, what we do here >>> now is a bit silly (while correct as it stands): The whole point >>> with those STL containers is that they provide an airtight and >>> convenient automatic memory management. However, we use that >>> automatically managed heap memory just to store pointers, >>> which we then manage manually as if it was just an array. >>> Thus we could consider to abandon using those pointers and >>> the explicit delete calls; rather we could just place our >>> payload directly into the heap memory managed by the container >>> and leave all the hard work to the compiler. >>> >>> However this might lead into a pitfall: since, after populating >>> this list, we add the created UI elements (the FilterLine instances) >>> to some other UI widget (the filerscroll), and we do so by pointer. >> I am not familiar with the code details, but the statement above >> seems to imply that FilterLine instances are /shared/ by a list of such >> instances and various filerscroll elements. If that is the case, then >> IMHO the list should accordingly contain shared elements, ie it >> should eg be of type std::vector <std::shared_ptr <FilerLine>>, and >> the various filerscroll instances should contain elements of type >> std::shared_ptr <FilerLine>. Sharing pointers directly is always a >> bad idea. > >I agree with Rainer, I think that switching to shared_ptr is far less >intrusive than the other suggested approaches, and is still both >idiomatic and safe. > Thanks everyone for coming in on this. With regard to vector v linked list, I did think about this, but it seems to me that although the list would setup faster, it would be slower to manage window resizes or object selections, so it seems a case of swings and roundabouts. Having said that, Hermann's code definitely looks cleaner, so I'll do some torture testing on it in a side branch, and if (as I expect) it's OK, we'll go with that for now. However, I think I should add a bit of background to the way this was developed. I based the overall structure on the way Paul originally set up part kits, and the mixer window - hence the fixed size array. I then took on Rainer's suggestion about making it a vector, which eventually worked. This is fine-ish if the original premise is correct, but from further 'net trawling I'm beginning to think Paul's original setup might have been wrong! It's very difficult to find any *really* clear info about FLTK, but the scroll class is described as a container, and if that means what I think it does, do we need the array/vector at all? Can we just dump the FilerLine objects into the scroll? If that is the case, then *all* the scroll elements in the code need revisiting, but I'm certainly not thinking of doing that before the next release! -- Will J Godfrey https://willgodfrey.bandcamp.com/ http://yoshimi.github.io Say you have a poem and I have a tune. Exchange them and we can both have a poem, a tune, and a song. _______________________________________________ Yoshimi-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/yoshimi-devel
