Flutter-geolocator: Wrap global functions to a static class

[kinex]

🏗 Enhancement Proposal

In the new version 6.0 you have introduced a set a global functions, for example checkPermission. Earlier it was Geolocator().checkPermission.

I think you have made a bad design mistake, earlier design was much better. Introducing a global functions named like checkPermission is a really bad idea. Names like checkPermission do not indicate any relation to geolocator, it could be related to anything like camera.

Practically your new API forces us to write import statement always something like:
import 'package:geolocator/geolocator.dart' as geolocator;

And then you can write geolocator.checkPermission().

But what's the point? Assuming no member data is needed, I suggest you wrap all those global functions inside a class as static methods. For example:

class Geolocator {
  Geolocator._();
  static Future<LocationPermission> checkPermission() =>
    GeolocatorPlatform.instance.checkPermission();
}

[mvanbeusekom]

In this case I followed the design of other popular Flutter plugins provided by the Flutter team (e.g. the "url_launcher" and "path_provider").

I see your point and will think about creating the wrapper class as an additional option (maybe add it to the documentation as recommendation). Just to keep things compatible I will keep the global methods around for now (possibly add the deprecated attribute). I will also be checking up with the Flutter team to hear their reasoning regarding the use of the global methods in their APIs.

[kinex]

I would say this same comment at least for "path_provider". In my opinion they should have created a static class e.g. "Path" (similar to C# Path class) , but it may be too late to fix that. It has already become a "standard" to import it as "p". Resulting code is not very clear, but I guess we all have already used to this syntax with this plugin.

With "url_launcher" a global function is more acceptable, because there is only a single function in that API and the name "launch" quite clearly describes its purpose.

If you wanted to keep global functions, the names should be more clear. For example "checkLocationPermission" instead of "checkPermission", "requestLocationPermission" instead of "requestPermission" etc. But at least my personal preference for this kind of API (a group of related functions) is to wrap the API to a static class.

Notice also that code completion works much better with a static class (you only have to remember the class name to see in editor what's available in the API). With global functions you have to open API documentation or the related source file.

[mvanbeusekom]

Deprecated global methods and added a static class in version 6.1.0

[giorgio79]

Was this change tested?

I am now getting an error

    //checkGeoLocatorService();
    Geolocator.getCurrentPosition(desiredAccuracy: LocationAccuracy.high).then(
      (Position geolocatorPosition) {
        print("we have geoLocatorPosition "); 

[        ] Try correcting the name to the name of an existing getter, or defining a getter or field named 'Geolocator'.
[        ]     Geolocator.getCurrentPosition(desiredAccuracy: LocationAccuracy.high).then(
[        ]     ^^^^^^^^^^

[kinex]

It works fine. Try flutter clean and check your import statements.

[mvanbeusekom]

Just to confirm the question "Was this change tested", yes it was. The example app also has been update and just to make sure I just quickly created the following app which worked without any problems:

lib/main.dart:

import 'package:flutter/material.dart';
import 'package:geolocator/geolocator.dart';

void main() {
  runApp(MyApp());
}

class MyApp extends StatelessWidget {
  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      home: GeoTestPage(title: 'Geolocator Test'),
    );
  }
}

class GeoTestPage extends StatefulWidget {
  GeoTestPage({Key key, this.title}) : super(key: key);

  final String title;

  @override
  _GeoTestPageState createState() => _GeoTestPageState();
}

class _GeoTestPageState extends State<GeoTestPage> {
  List<Position> _positions = <Position>[];

  void _getPosition() {
    Geolocator.getCurrentPosition()
      .then((pos) => setState(() => _positions.add(pos)));
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Text(widget.title),
      ),
      body: ListView.builder(
          itemCount: _positions.length,
          itemBuilder: (context, index) {
            final position = _positions[index];
            return ListTile(
              title: Text('Lat: ${position.latitude}, Lon: ${position.longitude}'),
              subtitle: Text('Time: ${position.timestamp}, Acc: ${position.accuracy}'),
            );
          },
      ),
      floatingActionButton: FloatingActionButton(
        onPressed: _getPosition,
        tooltip: 'Acquire position',
        child: Icon(Icons.add_location),
      ), // This trailing comma makes auto-formatting nicer for build methods.
    );
  }
}

pubspec.yaml:

name: geo_test
description: A new Flutter project.

# The following line prevents the package from being accidentally published to
# pub.dev using `pub publish`. This is preferred for private packages.
publish_to: 'none' # Remove this line if you wish to publish to pub.dev

# The following defines the version and build number for your application.
# A version number is three numbers separated by dots, like 1.2.43
# followed by an optional build number separated by a +.
# Both the version and the builder number may be overridden in flutter
# build by specifying --build-name and --build-number, respectively.
# In Android, build-name is used as versionName while build-number used as versionCode.
# Read more about Android versioning at https://developer.android.com/studio/publish/versioning
# In iOS, build-name is used as CFBundleShortVersionString while build-number used as CFBundleVersion.
# Read more about iOS versioning at
# https://developer.apple.com/library/archive/documentation/General/Reference/InfoPlistKeyReference/Articles/CoreFoundationKeys.html
version: 1.0.0+1

environment:
  sdk: ">=2.7.0 <3.0.0"

dependencies:
  flutter:
    sdk: flutter

  geolocator: ^6.1.0

  # The following adds the Cupertino Icons font to your application.
  # Use with the CupertinoIcons class for iOS style icons.
  cupertino_icons: ^0.1.3

dev_dependencies:
  flutter_test:
    sdk: flutter

# For information on the generic Dart part of this file, see the
# following page: https://dart.dev/tools/pub/pubspec

# The following section is specific to Flutter.
flutter:

  # The following line ensures that the Material Icons font is
  # included with your application, so that you can use the icons in
  # the material Icons class.
  uses-material-design: true

  # To add assets to your application, add an assets section, like this:
  # assets:
  #   - images/a_dot_burr.jpeg
  #   - images/a_dot_ham.jpeg

  # An image asset can refer to one or more resolution-specific "variants", see
  # https://flutter.dev/assets-and-images/#resolution-aware.

  # For details regarding adding assets from package dependencies, see
  # https://flutter.dev/assets-and-images/#from-packages

  # To add custom fonts to your application, add a fonts section here,
  # in this "flutter" section. Each entry in this list should have a
  # "family" key with the font family name, and a "fonts" key with a
  # list giving the asset and other descriptors for the font. For
  # example:
  # fonts:
  #   - family: Schyler
  #     fonts:
  #       - asset: fonts/Schyler-Regular.ttf
  #       - asset: fonts/Schyler-Italic.ttf
  #         style: italic
  #   - family: Trajan Pro
  #     fonts:
  #       - asset: fonts/TrajanPro.ttf
  #       - asset: fonts/TrajanPro_Bold.ttf
  #         weight: 700
  #
  # For details regarding fonts from package dependencies,
  # see https://flutter.dev/custom-fonts/#from-packages

android/app/src/main/AndroidManifest.xml:

<manifest xmlns:android="http://schemas.android.com/apk/res/android"
    package="com.example.geo_test">

    <uses-permission android:name="android.permission.ACCESS_FINE_LOCATION" />

    <!-- io.flutter.app.FlutterApplication is an android.app.Application that
         calls FlutterMain.startInitialization(this); in its onCreate method.
         In most cases you can leave this as-is, but you if you want to provide
         additional functionality it is fine to subclass or reimplement
         FlutterApplication and put your custom class here. -->
    <application
        android:name="io.flutter.app.FlutterApplication"
        android:label="geo_test"
        android:icon="@mipmap/ic_launcher">
        <activity
            android:name=".MainActivity"
            android:launchMode="singleTop"
            android:theme="@style/LaunchTheme"
            android:configChanges="orientation|keyboardHidden|keyboard|screenSize|smallestScreenSize|locale|layoutDirection|fontScale|screenLayout|density|uiMode"
            android:hardwareAccelerated="true"
            android:windowSoftInputMode="adjustResize">
            <!-- Specifies an Android theme to apply to this Activity as soon as
                 the Android process has started. This theme is visible to the user
                 while the Flutter UI initializes. After that, this theme continues
                 to determine the Window background behind the Flutter UI. -->
            <meta-data
              android:name="io.flutter.embedding.android.NormalTheme"
              android:resource="@style/NormalTheme"
              />
            <!-- Displays an Android View that continues showing the launch screen
                 Drawable until Flutter paints its first frame, then this splash
                 screen fades out. A splash screen is useful to avoid any visual
                 gap between the end of Android's launch screen and the painting of
                 Flutter's first frame. -->
            <meta-data
              android:name="io.flutter.embedding.android.SplashScreenDrawable"
              android:resource="@drawable/launch_background"
              />
            <intent-filter>
                <action android:name="android.intent.action.MAIN"/>
                <category android:name="android.intent.category.LAUNCHER"/>
            </intent-filter>
        </activity>
        <!-- Don't delete the meta-data below.
             This is used by the Flutter tool to generate GeneratedPluginRegistrant.java -->
        <meta-data
            android:name="flutterEmbedding"
            android:value="2" />
    </application>
</manifest>

[giorgio79]

Thanks guys, flutter clean solved it