Reputation: 1983
I get the Stack Overflow error when I have the following design:
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:
_timerNotifier.value
-> getIt<TimerContainerController>().value
_timerService.init()
-> getIt<TimerService>().init()
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
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.
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:
It's called a Controller Layer but it really is a Data Layer
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
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:
There should be a new controller that looks like this:
import 'package:minimalist_timer_app/widgets/timer_container/timer_notifier.dart';
class TimerContainerController {
final timerNotifier = getIt<TimerNotifier>();
init() => timerNotifier.init();
}
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).
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.
So, we have to remove the dependency (and also the import):
final _timerNotifier = getIt<TimerContainerController>();
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.
Instead, what the service should do, it should return something. So, let it return the new value like this:
String init() {
// ...
return new;
}
Fixing the wrong design decisions, I have found three different possible design solutions:
init()
: logic layer performs init()
final timerNotifier = ValueNotifier<String>();
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);
});
}
}
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();
}
}
class TimerService {
String init() {
// ...
return _newString;
}
}
init()
: also the logic layer performs init()
final timerNotifier = getIt<TimerContainerNotifier>();
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);
});
}
}
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();
}
}
import 'package:flutter/material.dart';
class TimerNotifier extends ValueNotifier<String> {
TimerNotifier() : super(mkDefaultTimerString);
}
class TimerService {
String init() {
// ...
return _newString;
}
}
init()
: data layer performs init()
final timerNotifier = getIt<TimerContainerNotifier>();
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);
});
}
}
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();
}
}
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();
}
}
class TimerService {
String init() {
// ...
return _newString;
}
}
Upvotes: 0
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):
class TimerContainerController extends ValueNotifier<String> {
TimerContainerController() : super(defaultTimerString);
init() {
final _timerService = getIt<TimerService>();
_timerService.init();
}
}
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.)
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,
),
);
},
);
}
}
import './timer_container_notifier.dart';
class TimerContainerController {
final timerNotifier = TimerContainerNotifier();
init() {
timerNotifier.init();
}
dispose() {
timerNotifier.dispose();
}
// ... various other action handlers from the UI
}
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
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
Reputation: 511746
Since you referenced my article, State Management for Minimalists, here are a few supplementary notes:
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.Upvotes: 1