Refactor: Simplify LayersService.addLayer

[MichaelLangbein]

Description

We're close to issuing a ukis-version 8.0. To this occasion, maybe cleaning up some places in the code could be useful. Last time we did that, there were just too many changes coming together at once, so here I'll try to handle things on a small-issue basis, so that things don't go out of hand.

Concretely, here is a potential chance for some cleaning up: in the method LayersService.addLayer, to we still need the toGroup parameter? I don't really understand toGroup anyway. I don't think that this method does anything when toGroup is given!

/**
   * Adds a ukis Layer to the Layerservice Store
   * filtertype: TFiltertypes
   * if filtertype is not provided the filtertype of the Layer is used!
   *
   * if toGroup is true the layer is not added to the list of Layers and storeItems. Only used  internally.
   */
  public addLayer(layer: Layer, filtertype?: TFiltertypes, toGroup?: boolean) {
    if (!this.isInLayergroups(layer)) {

      if (!filtertype) {
        filtertype = layer.filtertype;
      } else {
        // set filtertype of Layer!!
        layer.filtertype = filtertype;
      }

      const storeItems = this.store.getValue();

      if (toGroup) {
        this.filterFiltertype(filtertype);  // <-- updates baselayers, layers and overlays with store's current contents.
      } else {
        storeItems.push(layer);

        this.store.next(storeItems);
        this.filterFiltertype(filtertype);  // <-- updates baselayers, layers and overlays with store's current contents.
      }
    } else {
      console.error(`layer or Group with id: ${layer.id} already exists!`);
    }
  }

• Removing this parameter would make the code a lot more readable and understandable for new users.
• I've grepped for any usage of toGroups in the libraries and couldn't find any. Neither was it used in any ukis-project that I've ever worked on. I'm almost positive that this option is no longer used.

Relevant Package

This feature request is for @dlr-eoc/services-layers

[MichaelLangbein]

There is just a really tiny thing I'd like to also do here: rename filterFiltertype to updateAllLayersByStoreContent, just because it seems like a more fitting name, and also makes addLayers a bit more readable.

[boeckMt]

Yes you are completely right! I also checked it and we don't use the option toGroup on addLayer. I think it is also a relict from the beginning of the LayerService, where we tried to create only on function for adding layers to the Store and to a Group. Maybe @voinSR can also verify this?

Personally I would like to refactor the complete LayerService because there are so much clutter, but we really have to check that we are not breaking anything here because it is a core functionality of our library.

I also would like to make the Store more dynamic, so yo can add as much types of layers like you want.

---

In regards to filterFiltertype I would use something like updateStoreTypes, but since this is a private function we can rename it easily to anything.

[MichaelLangbein]

Awesome, thanks! I'll create a PR as soon as I can (hopefully this week).

[voinSR]

Maybe @voinSR can also verify this?

As @boeckMt mentioned, initially we had something like addLayer and addLayergroup... maybe it was intention to keep it in one method and use this option. In the fact we never used... i think

I agree, it's already time to refactor LayerService. Maybe we could keep old methods which would probably call the new ones internally and mark them as deprecated for a while. This way we may reduce impact of breaking changes

[boeckMt]

Ideas for Refactoring:

• Let the user know which layers have changed: It is currently possible to subscribe to changes in the store, but the user only knows that there has been a change, not which layers have changed. -> Change of attribute (updateLayerOrGroupInStore) or change of order (setLayerGroups) or change of length (setLayerGroups)

// layers.service.ts
private updateLayerOrGroupInStore(layerOrGroup: Layer | LayerGroup) {
    this.store.getValue().filter((lg, index, array) => {
+      if (lg instanceof Layer && lg.id !== layerOrGroup.id) {
+        lg.updated = false;
+      }
+      if (lg instanceof LayerGroup && lg.id !== layerOrGroup.id) {
+        lg.layers.forEach(l => l.updated = false);
+      }
      // check if both from the same type then check same id
      if (lg instanceof Layer && layerOrGroup instanceof Layer) {
        if (lg.id === layerOrGroup.id) {
          array[index] = layerOrGroup;
          this.store.next(array);
        }
      } else if (lg instanceof LayerGroup && layerOrGroup instanceof LayerGroup) {
        if (lg.id === layerOrGroup.id) {
          array[index] = layerOrGroup;
          this.store.next(array);
        }
      }
    });
+    // Do this afterwards, because referenced objects will be changed.
+    if (layerOrGroup instanceof Layer) {
+      layerOrGroup.updated = true;
+    }
+    if (layerOrGroup instanceof LayerGroup) {
+      layerOrGroup.layers.forEach(l => l.updated = true);
+    }
}

// Layers.ts
export class Layer implements ILayerOptions {
  ...
+  updated: boolean = false;
  ...
}

// layers.service.ts
+ public change = new Subject<'attribute' | 'order' | 'length'>()
+ // trigger change in add/Update/remove/set 

  constructor() {

  }

+  private getLengthChange(items: Array<Layer | LayerGroup>) {
+    const oldItemsLength = this.getLayerGroupsCount();
+    // TODO: layers in group changed?
+    if (oldItemsLength !== items.length) {
+      return true;
+    } else {
+      const oldItemsFlatLength = this.getBaseLayersCount() + this.getLayersCount() + this.getOverlaysCount();
+      const newItemsFlat = this.flattenDeepArray(items);
+      if (oldItemsFlatLength !== newItemsFlat.length) {
+        return true;
+      } else {
+        return false;
+      }
+    }
+  }

+  private setStore(items: Array<Layer | LayerGroup>) {
+    const changeLength = this.getLengthChange(items);
+    if (changeLength) {
+      this.change.next('length');
+    }
+    this.store.next(items);
+  }


- this.store.next(...);
+ this.setStore(...);


public setLayerGroups(items: Array<Layer | LayerGroup>, filtertype?: TFiltertypes): Observable<Array<Layer | LayerGroup>> {
    if (items.length > 0) {
    ...
+   const changeLength = this.getLengthChange(items);
+    if(!changeLength){
+      this.change.next('order');
+    }
    if (!filtertype) {
   ...

// layers.service.ts
public addLayerGroup(layergroup: LayerGroup, filtertype?: TFiltertypes) {
      ...
-      // update to set visible -> do we need this really
-       this.updateLayerGroup(layergroup);
    }
  }

---

• Simplify public methods for add/get/remove/update Layer and LayerGroup

---

• Allow more TFiltertypes : It would be nice if a user could specify different filter types to add to the existing ones and in wich order the can be pulled

[boeckMt]

Maybe we can check if some Observer utilities could help to synchronize updates to layers.

• https://github.com/nx-js/observer-util
• https://github.com/vuejs/core/tree/main/packages/reactivity