MainWindow: add and remove search results instead of clearing the list

[danirabbit]

Should allow us to take advantage of widget recycling in #147

Only clear the list on error. Otherwise, add or remove results

[ryonakano]

I guess GLib.List.find() is pointer comparison? So I'm not sure if this works as you expect.

[danirabbit]

Yeah I wasn't sure about it, but from testing it seems to work. I had it print number added and number removed and most of the time they were not equal amounts and there was matches to be removed 🤷‍♀️

[danirabbit]

Maybe find with equal func would be better?

[ryonakano]

As far as I understand and test correctly, all of old results are removed even if the data in the list is completely equal because they have different (memory) address.

I try calling compute_location() again immediately after calling it first time, which means Geocode.Forward.search_async() should return the same result in its content data:

user@elementary-8-daily:~/work/maps$ git diff
diff --git a/src/MainWindow.vala b/src/MainWindow.vala
index c0f5dcd..3bffebb 100644
--- a/src/MainWindow.vala
+++ b/src/MainWindow.vala
@@ -306,6 +306,9 @@ public class Maps.MainWindow : Adw.ApplicationWindow {
     private async void search_location (string term, ListStore res) {
         busy_start (BusyReason.SEARCHING);
 
+        warning ("Round 1 begin");
+        yield compute_location (term, res);
+        warning ("Round 2 begin");
         yield compute_location (term, res);
 
         busy_end (BusyReason.SEARCHING);
@@ -334,10 +337,12 @@ public class Maps.MainWindow : Adw.ApplicationWindow {
         // Remove any old results that aren't in the new set
         for (int i = 0; i < loc_store.n_items;) {
             if (places.find ((Geocode.Place) loc_store.get_item (i)) == null) {
+                warning ("Not in the new list, removing");
                 loc_store.remove (i);
                 continue;
             }
 
+            critical ("Still in the new list, keeping");
             i++;
         }
 
@@ -345,8 +350,12 @@ public class Maps.MainWindow : Adw.ApplicationWindow {
         foreach (unowned var place in places) {
             uint pos = -1;
             if (!loc_store.find (place, out pos)) {
+                warning ("Missing in the list, adding");
                 loc_store.append (place);
+                continue;
             }
+
+            critical ("Already in in the list!");
         }
     }
 
user@elementary-8-daily:~/work/maps$ 

The above code results that all results are removed:

** (io.elementary.maps:15415): WARNING **: 20:49:12.148: MainWindow.vala:309: Round 1 begin

** (io.elementary.maps:15415): WARNING **: 20:49:13.519: MainWindow.vala:353: Missing in the list, adding

** (io.elementary.maps:15415): WARNING **: 20:49:13.519: MainWindow.vala:353: Missing in the list, adding

** (io.elementary.maps:15415): WARNING **: 20:49:13.519: MainWindow.vala:353: Missing in the list, adding

** (io.elementary.maps:15415): WARNING **: 20:49:13.520: MainWindow.vala:311: Round 2 begin

** (io.elementary.maps:15415): WARNING **: 20:49:13.570: MainWindow.vala:340: Not in the new list, removing

** (io.elementary.maps:15415): WARNING **: 20:49:13.571: MainWindow.vala:340: Not in the new list, removing

** (io.elementary.maps:15415): WARNING **: 20:49:13.571: MainWindow.vala:340: Not in the new list, removing

** (io.elementary.maps:15415): WARNING **: 20:49:13.571: MainWindow.vala:353: Missing in the list, adding

** (io.elementary.maps:15415): WARNING **: 20:49:13.571: MainWindow.vala:353: Missing in the list, adding

** (io.elementary.maps:15415): WARNING **: 20:49:13.571: MainWindow.vala:353: Missing in the list, adding

[ryonakano]

Maybe find with equal func would be better?

@danirabbit Yes, you should use GLib.List.find_custom() instead if you really want to do this, but honestly I'm not sure if removing/adding results is better than clearing the liststore……I know this is an application code but we call find() in the list, which I don't think light call, twice in this code.

[danirabbit]

Okay gonna just close. This might be trying to over-optimize. We can always revisit it later if there's a real performance issue to look at