fluttercommunity / get_it

Get It - Simple direct Service Locator that allows to decouple the interface from a concrete implementation and to access the concrete implementation from everywhere in your App. Maintainer: @escamoteur
https://pub.dev/packages/get_it
MIT License
1.36k stars 149 forks source link

Concerns about Get_It #353

Closed warcayac closed 11 months ago

warcayac commented 11 months ago

I usually use Singletons, but I've read recommendations about using Get_It instead so I'm trying to give it a try. However, I have found a somewhat brief documentation on how to correctly use (through practical examples) each of the features offered by this package (a common bad practice of many packages). The tutorials that I have found on the Web are basically the same as what is shown in the Get_It documentation on Github, that is, they do not provide better knowledge about the use of the other features not exemplified in the original documentation. Even searching GitHub there are very few cases where unexemplified cases (like registerSingletonAsync) are addressed.

Part I

Let's look at the following demo app:

my.env

KEYA="Kentaro Miura"
KEYB="Berkerk, the best manga ever"

openweather.dart

import 'package:flutter_dotenv/flutter_dotenv.dart';

final class OpenWeather {
  final String keyA, keyB;
  const OpenWeather._({required this.keyA, required this.keyB});

  static OpenWeather? _instance;

  static Future<bool> init() async {
    try {
      if (_instance != null) return true;

      await dotenv.load(fileName: 'assets/env/my.env');
      _instance = OpenWeather._(
        keyA: dotenv.get('KEYA'),
        keyB: dotenv.get('KEYB'),
      );
      return true;
    } catch (e) {
      throw Exception('Env file cannot be loaded or some required field was not found.');
    }
  }

  static OpenWeather get instance => _instance != null 
    ? _instance!
    : throw Exception('Instance was not initialized')
  ;
}

anyclass.dart

class AnyClass {
  final String author, desc;

  AnyClass() : 
    author = OpenWeather.instance.keyA,
    desc = OpenWeather.instance.keyB
  ;

  @override
  String toString() => 'From AnyClass: $author [$desc]';
}

home_page.dart

import 'package:flutter/material.dart';

import '../../main.dart';
import '../../singleton/anyclass.dart';
import '../../singleton/openweather.dart';

class HomePage extends StatelessWidget {
  const HomePage({super.key});

  @override
  Widget build(BuildContext context) {
    final textTheme = Theme.of(context).textTheme;

    return Scaffold(
      appBar: AppBar(
        title: const Text('Get_It demo 2'),
      ),
      body: Center(
        child: FutureBuilder(
          future: allReady,
          builder: (context, snapshot) {
            if (snapshot.hasData) {
              final obj = AnyClass();

              return Column(
                mainAxisAlignment: MainAxisAlignment.center,
                children: [
                  const Text('From Singleton:'),
                  Text(OpenWeather.instance.keyA, style: textTheme.headlineMedium),
                  Text(OpenWeather.instance.keyB, style: textTheme.headlineSmall),
                  const SizedBox(height: 30),
                  Text(obj.toString()),
                ],
              );
            }

            return const CircularProgressIndicator();
          }
        ),
      ),
    );
  }
}

main.dart

import 'package:flutter/material.dart';

import 'pages/home/home_page.dart';
import 'singleton/openweather.dart';

late Future<bool> allReady;

void main() async {
  allReady = OpenWeather.init();
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Welcome to Flutter',
      debugShowCheckedModeBanner: false,
      theme: ThemeData(
        colorScheme: ColorScheme.fromSeed(seedColor: Colors.deepPurple),
        useMaterial3: true,
      ),
      home: const HomePage(),
    );
  }
}

This app runs as expected, I can even remove the allReady variable and the FutureBuilder widget, and the app still runs without problems.

main.dart

...
void main() async {
  await OpenWeather.init();
  runApp(const MyApp());
}
...

home_page.dart

class HomePage extends StatelessWidget {
  const HomePage({super.key});

  @override
  Widget build(BuildContext context) {
    final textTheme = Theme.of(context).textTheme;
    final obj = AnyClass();

    return Scaffold(
      appBar: AppBar(
        title: const Text('Get_It demo 2'),
      ),
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: [
            const Text('From Singleton:'),
            Text(OpenWeather.instance.keyA, style: textTheme.headlineMedium),
            Text(OpenWeather.instance.keyB, style: textTheme.headlineSmall),
            const SizedBox(height: 30),
            Text(obj.toString()),
          ],
        ),
      ),
    );
  }
}

Part II

Now I gonna change/add some files to use them with Get_It package:

openweather.dart

final class OpenWeather {
  final String keyA, keyB;

  OpenWeather({required this.keyA, required this.keyB});

  static Future<OpenWeather> init() async {
    try {
      await dotenv.load(fileName: 'assets/env/my.env');
      return OpenWeather(
        keyA: dotenv.get('KEYA'),
        keyB: dotenv.get('KEYB'),
      );
    } catch (e) {
      throw Exception('Env file cannot be loaded or some required field was not found.');
    }
  }
}

di.dart

import 'package:get_it/get_it.dart';

import '../singleton/openweather.dart';

class GIt {
  const GIt._();

  static Future<void> configureDependencies() async {
    GetIt.I
      .registerSingletonAsync<OpenWeather>(OpenWeather.init)
    ;
  }

  static Future<void> allReady() => GetIt.I.allReady();

  static Future<OpenWeather> get openWeather => GetIt.I.getAsync<OpenWeather>();
}

main.dart

void main() async {
  GIt.configureDependencies();
  runApp(const MyApp());
}

home_page.dart

class HomePage extends StatelessWidget {
  const HomePage({super.key});

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: const Text('Get_It demo 2'),
      ),
      body: FutureBuilder(
        future: GIt.openWeather,
        builder: (context, snapshot) {
          return snapshot.hasData 
            ? Column(
                mainAxisAlignment: MainAxisAlignment.center,
                children: [
                  Text(snapshot.data!.keyA, style: Theme.of(context).textTheme.headlineMedium),
                  Text(snapshot.data!.keyB, style: Theme.of(context).textTheme.headlineSmall),
                ],
              )
            : const Center(child: CircularProgressIndicator())
          ;
        }
      ),
    );
  }
}

Observations


Part III

  1. Due to the lack of better documentation and/or my programming skills, the use of the package in this simple demo makes the code more complex.
  2. It seems the main goal of this package is for test purposes only, according to its documentation, by adding certain complexity into the code, so traditional singletons still keep advantage for many cases, IMHO
  3. this makes injectable package even less attractive to use

If any assessment is wrong I would appreciate appropriate guidance.

escamoteur commented 11 months ago

Hi,

first of all I can assure you, this package is not for test purposes but used by thousands of Flutter devs in production. I don't understand why you wrap the GetIt functions in a class with only static functions. Why not just assign the GetIt.I to a global variable or just use GetIt.I whenever you want to access get_it?

If you only have one async init function, then it's totally ok to await that before starting the app. However using registerSingletonAsync and then use allReady in you futurehandler ensures that your apps UI is already started while your async function might wait for some network response. (isReady is only needed in very special settings)

I recommend watching my talk https://youtu.be/YJ52kSfSMyM?si=Mia_0usLJJ1B_t9M to see how allReady and registerAsyncSingleton can make your life easier.

Sure you can just use normal Singletons but then you don't have the option to switch implementations at runtime like I do here:

  if (Platform.isIOS || Platform.isAndroid) {
    di.registerSingleton<LocalNotificationService>(
        LocalNotificationServiceImpl());
  } else {
    di.registerSingleton<LocalNotificationService>(
        LocalNotificationServiceDesktop());
  }

Also you won't have scopes which can be very handy.

Btw. injectable isn't from me and was developed without my involvement,

escamoteur commented 11 months ago

You might also have a look at my watch_it package wich is the state management extension for get_it

warcayac commented 11 months ago

I advocate practicality in my apps, so instead of adding two import sentences (one for Get_It and one for my class) I prefer one import sentence through a static class called GIt which works as your global variable getIt but much better, IMHO.

I've watched your video and I've understood your package a little better, however I'd suggest you generate practical examples for every functionality your package offers, that would help more. As the saying goes "more action and fewer words".

Anyway, thanks for your time and happy X-mas and happy new year.

escamoteur commented 11 months ago

Actually if you put all your registrations in one file you only have to import that one in all other files Am 28. Dez. 2023, 00:12 +0100 schrieb William Arcaya C. @.***>:

I advocate practicality in my apps, so instead of adding two import sentences (one for Get_It and one for my class) I prefer one import sentence through a static class called GIt which works as your global variable getIt but much better, IMHO. I've watched your video and I've understood your package a little better, however I'd suggest you generate practical examples for every functionality your package offers, that would help more. As the saying goes "more action and fewer words". Anyway, thanks for your time and happy X-mas and happy new year. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>