Tomas Baran
Tomas Baran

Reputation: 1983

GetIt plugin Stack Overflow Error when two classes depend on each other

I get the Stack Overflow error when I have the following design:

enter image description here

I get that the problem is the first dotted arrow.

Here it is in the code:

import 'package:flutter/material.dart';
import 'package:minimalist_timer_app/widgets/timer_container/timer_container_controller.dart';
import 'package:minimalist_timer_app/services/service_locator.dart';

class TimerContainer extends StatefulWidget {
  const TimerContainer({Key? key}) : super(key: key);

  @override
  State<TimerContainer> createState() => _TimerContainerState();
}

class _TimerContainerState extends State<TimerContainer> {
  @override
  void initState() {
    super.initState();
    _widgetController.init();
  }

  final TimerContainerController _widgetController = getIt<TimerContainerController>();
  @override
  Widget build(BuildContext context) {
    return ValueListenableBuilder<String>(
        valueListenable: _widgetController,
        builder: (_, timer, __) {
          return Text(
            timer,
            style: const TextStyle(fontSize: 70, fontWeight: FontWeight.w700),
          );
        });
  }
}
import 'package:flutter/material.dart';
import 'package:minimalist_timer_app/services/service_locator.dart';
import 'package:minimalist_timer_app/services/timer_service.dart';

class TimerContainerController extends ValueNotifier<String> {
  TimerContainerController() : super(defaultTimerString);
  final _timerService = getIt<TimerService>();

  init() => _timerService.init();
}
import 'package:flutter/material.dart';
import 'package:minimalist_timer_app/services/service_locator.dart';
import 'package:minimalist_timer_app/widgets/timer_container/timer_container_controller.dart';

class TimerService {
  final _timerNotifier = getIt<TimerContainerController>();

  init() {
    // ...
    _timerNotifier.value = _newString;
  }
}

I also found a workaround: If I don't declare _timerNotifier or _timerService in one of the files, the issue is gone, then I have to change my code like so:

So, in other words both cannot be declared at the same time since they target each other.

The question is, what is the best practice here? Is my workaround the way to go (not declaring the variable)? Or is my design wrong? If so, how shall I change it?

Upvotes: 1

Views: 1343

Answers (3)

Tomas Baran
Tomas Baran

Reputation: 1983

Disclaimer: Big shot out to @Abion47. Thanks to his direction, time, dedication and care I was able to get to the solution. So, the ideas were coming mostly from @Abion47 — not me.

PROBLEMS

The design is wrong. The circular dependency bug appears which wouldn't have appeared if the design had been done right.

The workaround works, however, it's better to change the design in the first place. Unless the design is changed, this kind of bug will keep occurring.

The incorrect design decisions are:

  • A. Mixing Data and Logic layer
  • B. Service layer is not really a service in the example

A. Mixed Data and Logic layer

  1. It's called a Controller Layer but it really is a Data Layer

  2. Following the previous point, therefore instead of using the TimerContainerController as a class and file name, it would be more appropriate to name it as TimerNotifier

  3. Based on the previous points, it's clear that the controller and the data layer were mixed as one layer (called controller layer) while their concern should be separated:

    • Data layer worries only about the data (with the exception of indirect invocations such as callbacks, streams, services)
    • Logic layer (controller) worries only about the logic
  4. There should be a new controller that looks like this:

  • timer_container_controller.dart
import 'package:minimalist_timer_app/widgets/timer_container/timer_notifier.dart';

class TimerContainerController {
  final timerNotifier = getIt<TimerNotifier>();

  init() => timerNotifier.init();
}

B. Service layer is not really a service

  1. Services are independent and should have no tight coupling to any other files/classes outside their own (imagine a web api). They don't depend on any app file (there should be no imports that make them depended on other data/file).

  2. With the 1. point being said, the service does not know, nor does it care who calls it. Service does its job and return whatever it needs to return to whoever it called it.

  3. So, we have to remove the dependency (and also the import): final _timerNotifier = getIt<TimerContainerController>();

  4. Since its independent, it cannot be changing data in other file/layer directly like it does here: _timerNotifier.value = new;, therefore it must be deleted, as well.

  5. Instead, what the service should do, it should return something. So, let it return the new value like this:

String init() {
    // ...
    return new;
}

SOLUTIONS

Fixing the wrong design decisions, I have found three different possible design solutions:

Solution A1

  • init(): logic layer performs init()
  • data layer: separated in the variable final timerNotifier = ValueNotifier<String>();

enter image description here

  • timer_container.dart (UI layer)
import 'package:flutter/material.dart';
import 'package:minimalist_timer_app/widgets/timer_container/timer_container_controller.dart';
import 'package:minimalist_timer_app/services/service_locator.dart';

class TimerContainer extends StatefulWidget {
  const TimerContainer({Key? key}) : super(key: key);

  @override
  State<TimerContainer> createState() => _TimerContainerState();
}

class _TimerContainerState extends State<TimerContainer> {
  final TimerContainerController _widgetController = getIt<TimerContainerController>();

  @override
  void initState() {
    super.initState();
    _widgetController.init();
  }

  @override
  Widget build(BuildContext context) {
    return ValueListenableBuilder<String>(
        valueListenable: _widgetController.timerNotifier,
        builder: (_, timer, __) {
          return Text(timer);
        });
  }
}
  • timer_container_controller.dart (logic + data layer)
import 'package:minimalist_timer_app/services/service_locator.dart';
import 'package:minimalist_timer_app/services/timer_service.dart';

class TimerContainerController {
  final timerNotifier = ValueNotifier<String>(mkDefaultTimerString);
  final _timerService = getIt<TimerService>();

  init() {
    timerNotifier.value = _timerService.init();
  }
}
  • timer_service.dart (service layer)

class TimerService {

  String init() {
    // ...
    return _newString;
  }
}

Option A2

  • very similar to the A1
  • init(): also the logic layer performs init()
  • data layer (difference from A1): extracted to a new file/class: final timerNotifier = getIt<TimerContainerNotifier>();

enter image description here

  • timer_container.dart (UI layer)
import 'package:flutter/material.dart';
import 'package:minimalist_timer_app/widgets/timer_container/timer_container_controller.dart';
import 'package:minimalist_timer_app/services/service_locator.dart';

class TimerContainer extends StatefulWidget {
  const TimerContainer({Key? key}) : super(key: key);

  @override
  State<TimerContainer> createState() => _TimerContainerState();
}

class _TimerContainerState extends State<TimerContainer> {
  final TimerContainerController _widgetController = getIt<TimerContainerController>();

  @override
  void initState() {
    super.initState();
    _widgetController.init();
  }

  @override
  Widget build(BuildContext context) {
    return ValueListenableBuilder<String>(
        valueListenable: _widgetController.timerNotifier,
        builder: (_, timer, __) {
          return Text(timer);
        });
  }
}
  • timer_container_controller.dart (logic layer)
import 'package:minimalist_timer_app/widgets/timer_container/timer_notifier.dart';
import 'package:minimalist_timer_app/services/service_locator.dart';
import 'package:minimalist_timer_app/services/timer_service.dart';

class TimerContainerController {
  final timerNotifier = ValueNotifier<String>(mkDefaultTimerString);
  final _timerService = getIt<TimerService>();

  init() {
    timerNotifier.value = _timerService.init();
  }
}
  • timer_container_notifier.dart (data layer)
import 'package:flutter/material.dart';

class TimerNotifier extends ValueNotifier<String> {
  TimerNotifier() : super(mkDefaultTimerString);
}
  • timer_service.dart (service layer)

class TimerService {

  String init() {
    // ...
    return _newString;
  }
}

Option B

  • init(): data layer performs init()
  • data layer: extracted to a new file/class: final timerNotifier = getIt<TimerContainerNotifier>();

enter image description here

  • timer_container.dart (UI layer)
import 'package:flutter/material.dart';
import 'package:minimalist_timer_app/widgets/timer_container/timer_container_controller.dart';
import 'package:minimalist_timer_app/services/service_locator.dart';

class TimerContainer extends StatefulWidget {
  const TimerContainer({Key? key}) : super(key: key);

  @override
  State<TimerContainer> createState() => _TimerContainerState();
}

class _TimerContainerState extends State<TimerContainer> {
  final TimerContainerController _widgetController = getIt<TimerContainerController>();

  @override
  void initState() {
    super.initState();
    _widgetController.init();
  }

  @override
  Widget build(BuildContext context) {
    return ValueListenableBuilder<String>(
        valueListenable: _widgetController.timerNotifier,
        builder: (_, timer, __) {
          return Text(timer);
        });
  }
}
  • timer_container_controller.dart (logic layer)
import 'package:minimalist_timer_app/widgets/timer_container/timer_notifier.dart';
import 'package:minimalist_timer_app/services/service_locator.dart';


class TimerContainerController {
  final timerNotifier = ValueNotifier<String>(mkDefaultTimerString);

  init() {
    timerNotifier.init();
  }
}
  • timer_container_notifier.dart (data layer)
import 'package:flutter/material.dart';
import 'package:minimalist_timer_app/services/timer_service.dart';

class TimerNotifier extends ValueNotifier<String> {
  TimerNotifier() : super(mkDefaultTimerString);

  init() {
    value = _timerService.init();
  }
}
  • timer_service.dart (service layer)

class TimerService {

  String init() {
    // ...
    return _newString;
  }
}

Upvotes: 0

Abion47
Abion47

Reputation: 24671

Best practice is to not have circular dependencies. There is no good clean way to do what you are doing, so the best thing to do is to redesign your architecture to not have this problem in the first place.

If for whatever reason you need to do something like this, move the getIt call to the method where you actually need the service instead of trying to cache the value in a field initializer or constructor (which is defeating the purpose of get_it anyway):

  • timer_container_controller.dart
class TimerContainerController extends ValueNotifier<String> {
  TimerContainerController() : super(defaultTimerString);

  init() {
    final _timerService = getIt<TimerService>();
    _timerService.init();
  }
}
  • timer_service.dart
class TimerService {
  init() {
    // ...

    final _timerNotifier = getIt<TimerContainerController>();
    _timerNotifier.value = _newString;
  }
}

But again, and I cannot stress this enough, the fact that you have to do this is an indicator of code smell, and you'd be better off using a different approach entirely in which two separate classes don't depend on each other for their initialization.


EDIT:

You still aren't fully grasping the meaning behind separation of logic and data, so I'm going to put this as an edit to this answer in the hopes that I can paint the picture perfectly clearly without a character limit and limited formatting.

In your answer, option A is really just more of the same of what you are doing, albeit with the circular dependency removed. You still have one class TimerController pulling double duty as handling both the logic and data. This violates single-responsibility as every class should only have one job.

Option B is a step in the right direction and more closely resembles what Suragch's example was doing in their article, but you still haven't grasped the concept behind dependency-inversion or what the purpose of a service is.

If you imagine a hierarchy of dependency, UI is at the top and logic is under UI so UI depends on the logic but logic does not particularly care about the UI. Below that is data where logic (and by extension UI) depends on the data but the data does not care about either the logic or the UI. In this hierarchy, you cannot have any layer directly invoking anything in a layer above it, and doing so violates dependency-inversion*. For example, you cannot have the data layer directly calling functions on the logic layer.

Now imagine services even lower in that hierarchy. Services by definition are responsible for their own operations and nothing else. You cannot have them depend on any other layer and you cannot have them directly invoking members of other objects as that defeats the entire purpose of a service being a service.


Another source of confusion I think is coming from the difference between "direct" and "indirect" invocation. Direct invocation means that code A is directly invoking a member of class B, as in it has an instance of that class and is calling the member explicitly. Indirect invocation means that the invocation is happening implicitly, as in code A might be calling a member of class B but it is not doing so directly via an instance of class B. Here are some practical examples:

// Direct invocations
final foo = new XYZ();
foo.a = 'something';
foo.doSomething();

final bar = getIt<Bar>();
final value = bar.getAValue();

// Indirect invocations
void useCallback(Function func) {
  func('some value');
}

void useStream() {
  this.streamController.add('some value');
}

void useNotifier() {
  this.value = 'some value';
}

Notice how all the direct invocations involve an actual object that is being called, whether the object came from manual construction or a service locator call. This is what makes the invocation direct - the code is directly invoking some member of that object. By contrast, indirect invocation does not explicitly invoke a member of some object, but rather invokes some function or sends off data with no assumptions of what the recipient is.

There are several different types of indirect invocation, and the example shows a few:

  • Callbacks are functions that sources can provide as a sort of "call this function when something relevant happens". In this example, whatever code calls the useCallback method is responsible for providing the callback. The useCallback method itself? It doesn't care where the callback came from. It just calls it.

  • Streams are sort of like continuous callbacks while also on steroids. The code responsible for handling the stream and sending data into the stream doesn't care what might be listening on the other end - or even if nothing is listening at all. It just pushes data in one end and goes about its day.

  • ValueNotifier indirectly invokes widgets to rebuild themselves by providing an event bus for widgets to optionally subscribe to. It doesn't go to each widget individually and manually tell it to rebuild, it just fires the event and leaves it up to listening widgets to react.


With these in mind, here is how I would structure your code:

(Note: I don't know what your end goal with some of these design decisions are, so if I were to know more about your end goals it's very possible that I would go in an entirely different direction. For example, I have no idea what the purpose of the string in your timer service is, but if it's supposed to be the current value of the timer, I would suggest leaving it as a number and letting the listener be responsible for converting it to a string.)

  • timer_container.dart
import 'package:flutter/material.dart';
import './timer_container_controller.dart';

class TimerContainer extends StatefulWidget {
  const TimerContainer({Key? key}) : super(key: key);

  @override
  State<TimerContainer> createState() => _TimerContainerState();
}

class _TimerContainerState extends State<TimerContainer> {
  final _widgetController = getIt<TimerContainerController>();

  @override
  void initState() {
    super.initState();
    _widgetController.init();
  }

  @override
  void dispose() {
    _widgetController.dispose();
    super.dispose();
  }

  @override
  Widget build(BuildContext context) {
    return ValueListenableBuilder<String>(
      valueListenable: _widgetController.timerNotifier,
      builder: (_, timer, __) {
        return Text(
          timer,
          style: const TextStyle(
            fontSize: 70, 
            fontWeight: 
            FontWeight.w700,
          ),
        );
      },
    );
  }
}
  • timer_container_controller.dart
import './timer_container_notifier.dart';

class TimerContainerController {
  final timerNotifier = TimerContainerNotifier();

  init() {
    timerNotifier.init();
  }

  dispose() {
    timerNotifier.dispose();
  }

  // ... various other action handlers from the UI
}
  • timer_container_notifier.dart
import './services/timer_service.dart';

class TimerContainerNotifier extends ValueNotifier<String> {
  timerService = TimerService(); // Or alternatively, `getIt<TimerService>()`

  TimerContainerNotifier() : super(mkDefaultTimerString);

  void init() {
    timerService.init(value);
    timerService.onTick = onTimerTick;
  }

  void dispose() {
    // Whatever clean-up necessary here
    // possibly `timerService.stop`
  }

  void onTimerTick(String newValue) {
    value = newValue;
  }

  // ... Various other data events
  • timer_service.dart
class TimerService {
  String timerValue; // Shouldn't this be a number, though?
  void Function(String) onTick;

  init(String initialValue) {
    _timerValue = initialValue;
  }

  // ... Various other timer actions
}

An easy way to see what class depends on what is to look at the imports. See how the container imports the controller, the controller imports the notifier, the notifier imports the service, and the service has no imports at all? That is the simple hierarchy that reflects the ideal separation between UI, logic, data, and services.


*: This is a bit of an oversimplification of the dependency-inversion principle as it pertains to the minimalistic approach. In reality, strict adherence to dependency-inversion means nothing directly depends on anything else, but rather dependencies are attached to loose abstractions that the source class provides implementations for. This is a bit overkill for a simple app, though, and is also one of those things that has rapidly diminishing returns of usefulness once you get past a certain amount of adherence.

Upvotes: 4

Suragch
Suragch

Reputation: 511746

Since you referenced my article, State Management for Minimalists, here are a few supplementary notes:

  • The service layer shouldn't know anything about the state management layer. The ValueNotifier is part of the state management layer and your TimerService shouldn't have a reference to it. Instead, your state management class (for you that's TimerContainerController) should call some method on the service layer and based on the data it gets back, the state management class will update the ValueNotifier. Think of the service layer like a web server. Anyone can request data from a web server, but that web server doesn't know anything about the web clients that are accessing it. This type of architecture will also prevent circular dependencies.
  • I tend to agree with Abion47 that "Controller" isn't the best name for the state management class for the reasons they mentioned. I've been calling mine "Manager" recently. But as long as you have the architecture right, the name doesn't matter that much.

Upvotes: 1

Related Questions