Code review for flexible geolocation provider

Ionic Framework ionic-v3
1

I’m new to Ionic and I’m buildling an app for both the browser and mobile. It relies on mapping so I’m writing a flexible Location provider to fetch the users location.

The idea is:

  • Get location from native Geolocation plugin if on a device
  • Get location from the browser navigator.geolocation web API if in the browser
  • Get location from an API endpoint (GeoIP) failing the above (not implemented yet)
  • Default to last know position failling the above
  • Default to a hard-coded value failing the above

I’d like to make sure I’m not overlooking anything obvious in the Ionic/Angular world, so I’d love some feedback on the approach I’m taking below.

import { Injectable } from '@angular/core';
import { Geolocation } from '@ionic-native/geolocation';
import { Platform } from 'ionic-angular';
import { Storage } from '@ionic/storage';

import { Connection } from './../../providers/connection/connection';

@Injectable()
export class Location {

  constructor(private platform: Platform, private geolocation: Geolocation,
              private storage: Storage, private connection: Connection) {
  }

  // Get location using Ionic Native Geolocation plugin
  private getDeviceLocation(): Promise<any> {
    return new Promise((resolve, reject) => {
      this.geolocation
        .getCurrentPosition()
        .then((pos) => {
          resolve({
            lat: pos.coords.latitude,
            lng: pos.coords.longitude,
          })
        })
        .catch(reject);
    });
  }

  // Get location using the browser API
  private getBrowserLocation(): Promise<any> {
    return new Promise((resolve, reject) => {
      if ('geolocation' in navigator) {
        navigator.geolocation.getCurrentPosition(
            (pos) => {
              resolve({
                lat: pos.coords.latitude,
                lng: pos.coords.longitude
              });
            },
            reject,
            {
              enableHighAccuracy: true,
              timeout: 5000,
              maximumAge: 0
            });
      } else {
        reject("Geolocation not available for this browser");
      }
    });
  }

  // Get location by calling external GeoIP API 
  private getGeoIPLocation(): Promise<any> {
    return new Promise((resolve, reject) => {
      if (this.connection.online) {
        reject('Not implemented');
      } else {
        reject("GeoIP location not available when offline");
      }
    });
  }

  // Get or set default location from storage
  private getStoredLocation(): Promise<any> {
    return new Promise((resolve, reject) => {
      Promise
        .all([
          this.storage.get('lat'),
          this.storage.get('lng')
        ])
        .then((values) => {
          let lat = values[0];
          let lng = values[1];
          if (!lat || !lng) {
            // TODO:
            // - can we get region information from device here?
            // - where do we store the default fallback lat/lng
            console.warn("No location saved in storage, fetching default location");
            lat = 53.350140;
            lng = -6.266155;
          }
          resolve({
            lat,
            lng
          });
        })
        .catch(reject)
    });
  }

  public getLocation(): Promise<any> {
    return new Promise((resolve, reject) => {

      let saveCoords = (coords) => {
        this.storage.set('lat', coords.lat);
        this.storage.set('lat', coords.lng);
        resolve(coords);
      }

      let getGeoLocation = this.platform.is('cordova') ? this.getDeviceLocation() : this.getBrowserLocation();
      getGeoLocation
        .then(saveCoords)
        .catch((err) => {
          console.warn('Device/browser geolocation failed, falling back to GeoIP. Reason:', err);
          this.getGeoIPLocation()
            .then(saveCoords)
            .catch((err) => {
              console.warn("GeoIP location failed, falling back to storage. Reason:", err);
              this.getStoredLocation()
                .then(saveCoords)
                .catch(reject);
            })
        });
    });
    
  }
2

Good Job. It will be great to check previously if GPS is enabled

3

I see no reason to be manually instantiating any of these Promises. See here for background.