6

Some time ago, I started to refactor my code of the main project, decoupling the business logic from controllers to services, according to guidelines. Everything went well, until I faced the problem of circular dependency (CD). I read some resources about this problem:

Question 1 on Stack Overflow

Question 2 on Stack Overflow

Miško Hevery blog

Unfortunately, for me it is not clear now, how can I solve the problem of CD for my project. Therefore, I prepared a small demo, which represents the core features of my project:

Link to Plunker

Link to GitHub

Short description of components:

  • gridCtrl Does not contain any business logic, only triggers dataload and when the data is ready, show grid.
  • gridMainService This is the main service, which contains object gridOptions. This object is a core object of grid, contains some api, initialized by framework, row data, columns headers and so on. From this service, I was planning to control all related grid stuff. Function loadGridOptions is triggerd by gridCtrl and waits until the row data and column definitions are loaded by corresponding services. Then, it initializes with this data gridOptions object.
  • gridConfigService This is simple service, which used to load column definitions. No problem here.
  • gridDataService This service used to load row data with function loadRowData. Another feature of this service: it simulates the live updates, coming from server ($interval function). Problem 1 here!
  • gridSettingsService This service is used to apply some settings to the grid(for example, how the columns should be sorted). Problem 2 here!

Problem 1: I have a circular dependency between gridMainService and gridDataService. First gridMainService use gridDataService to load row data with function:

self.loadRowData = function () {
    // implementation here
}

But then, gridDataService receives the updates from server and has to insert some rows into grid. So, it has to use gridMainService:

$interval(function () {

    var rowToAdd = {
      make: "VW " + index,
      model: "Golf " + index,
      price: 10000 * index
     };

     var newItems = [rowToAdd];

     // here I need to get access to the gridMainService
     var gridMainService = $injector.get('gridMainService');
     gridMainService.gridOptions.api.addItems(newItems);

     index++;

 }, 500, 10);

So, here I faced first CD.

Problem 2: I have a circular dependency between gridMainService and gridSettingsService. First, gridMainService triggering, when the grid is initially loaded and then it sets up default settings by the following function:

self.onGridReady = function () {
    $log.info("Grid is ready, apply default settings");
    gridSettingsService.applyDefaults();
};

But, to make some changes, gridSettingsService need an access to the gridMainService and its object gridOptions in order to apply settings:

 self.applyDefaults = function() {
     var sort = [
       {colId: 'make', sort: 'asc'}
     ];

     var gridMainService = $injector.get('gridMainService');
     gridMainService.gridOptions.api.setSortModel(sort);
 };

Question: How can I solve these CD cases in proper way? Because, Miško Hevery Blog was quite short and good, but I have not managed to apply his strategy to my case.

And currently, I don't like an approach of manual injection a lot, because I will have to use it a lot and the code looks a little bit fuzzy.

Please note: I prepared only a demo of my big project. You can probably advice to put all code in gridDataService and ready. But, I already has a 500 LOC in this service, if I merge all services, it will be a ~1300 LOC nightmare.

Community
  • 1
  • 1
omalyutin
  • 445
  • 7
  • 24
  • I am not very good at it but I am not pretty sure about the reason of having two different services just to handle the data. Though you have mentioned their duties, still in my opinion, since you are handling grid data in both of them, you can have them in one service itself. I might be missing some piece of logic here. – PM. Jan 09 '17 at 21:45
  • Please allow me to say that you went full insane with the decoupling, you are giving yourself an unnecessary headache. First, I do not agree that grid management is something that a service should handle, a service is meant to handle pure business logic, and not dictate how a grid should behave, since that's the view thus a job for the controller. Second, if you still believe you need services for that, I'd highly suggest you simply have all functionality in one service, thus avoiding the CD nonsense. – M0nst3R Jan 09 '17 at 22:27
  • Reduce the number of services and decouple data from config. Initialize the grid with empty data and update it when it is ready. – kuhnroyal Jan 10 '17 at 18:42
  • @Ghassen Louhaichi I can probably agree with the first point, placing the grid object into the controller. Good point, thanks, I will try to make some examples using this idea. However, I would like to strongly disagree with the second point. As I have mentioned in the last paragraph, if I merge the services from my original project, there will be one file with around 800-1300 LOC. I think it will be messy, hard to test, hard to maintain, hard to understand, what is going on in this service. – omalyutin Jan 14 '17 at 13:32
  • If your project is complex enough, you run the inevitable risk of messy/spaghetti code, but that can be fixed by making your code well structured and greatly commented. I think that should be the way to go because most of the time, if you run into a circular dependency, your design is probably wrong. – M0nst3R Jan 14 '17 at 14:01
  • @CodeMonkey The plunker you linked does not present the CD issues you mentioned. – GPicazo Jan 14 '17 at 17:06
  • @GPicazo yes, because otherwise the example will not run at all. Therefore, I have manually injected _gridMainService_ in two places, which I have mentioned in the question. – omalyutin Jan 14 '17 at 17:27
  • @CodeMonkey - I've submitted a possible solution. Let me know your thoughts on it. – GPicazo Jan 14 '17 at 19:18

3 Answers3

2

In these kind of problems there are many solutions, which depend by the way you are thinking. I prefer thinking each service (or class) has some goal and needs some other service to complete its goal, but its goal is one, clear and small. Let’s see your code by this view.

Problem 1:

GridData: Here lives the data of grid. MainService comes here to get the data that needs, so we inject this to mainService and we use the loadRowData function to get the rowData data as you do, but in $interval you inject mainService inside gridData but gridData doesn’t need mainService to end its goal (get items from server).

I solve this problem using an observer design pattern, (using $rootScope). That means that I get notified when the data are arrived and mainService come and get them.

grid-data.service.js :

angular.module("gridApp").service("gridDataService",
         ["$injector", "$interval", "$timeout", "$rootScope",
 function ($injector, $interval, $timeout, $rootScope) {
   […]
   $interval(function () {
   [..]
   self.newItems = [rowToAdd];

    // delete this code
    // var gridMainService = $injector.get('gridMainService'); 
    // gridMainService.gridOptions.api.addItems(newItems);

    // notify the mainService that new data has come!
    $rootScope.$broadcast('newGridItemAvailable');

grid-main.service.js:

 angular.module("gridApp").service("gridMainService",
          ["$log", "$q", "gridConfigService", "gridDataService", '$rootScope',
  function ($log, $q, gridConfigService, gridDataService, $rootScope) {
     [..]
     // new GridData data  arrive go to GridData to get it!
     $rootScope.$on('newGridItemAvailable', function(){
        self.gridOptions.api.addItems(gridDataService.getNewItems());
     })
     [..]

When a real server is used, the most common solution is to use the promises (not observer pattern), like the loadRowData.

Problem 2:

gridSettingsService: This service change the settings of mainService so it needs mainService but mainService doesn’t care about gridSettings, when someone wants to change or learn mainService internal state (data, form) must communicate with mainService interface.

So, I delete grid Settings injection from gridMainService and only give an interface to put a callback function for when Grid is Ready.

grid-main.service.js:

angular.module("gridApp").service("gridMainService",
          ["$log", "$q", "gridConfigService", "gridDataService", '$rootScope',
 function ($log, $q, gridConfigService, gridDataService, $rootScope) {
   […]
   // You want a function to run  onGridReady, put it here!
   self.loadGridOptions = function (onGridReady) {
          [..]
          self.gridOptions = {
              columnDefs: gridConfigService.columnDefs,
              rowData: gridDataService.rowData,
              enableSorting: true,
              onGridReady: onGridReady // put callback here
            };
          return self.gridOptions;
        });
[..]// I delete the onGridReady function , no place for outsiders
    // If you want to change my state do it with the my interface

Ag-grid-controller.js:

    gridMainService.loadGridOptions(gridSettingsService.applyDefaults).then(function () {
      vm.gridOptions = gridMainService.gridOptions;
      vm.showGrid = true;
});

here the full code: https://plnkr.co/edit/VRVANCXiyY8FjSfKzPna?p=preview

Thomas Karachristos
  • 3,237
  • 18
  • 22
1

You can introduce a separate service that exposes specific calls, such as adding items to the grid. Both services will have a dependency to this api service, which allows for the data service to drop its dependency on the main service. This separate service will require your main service to register a callback that should be used when you want to add an item. The data service in turn will be able to make use of this callback.

angular.module("gridApp").service("gridApiService", function () {

        var self = this;

        var addItemCallbacks = [];

        self.insertAddItemCallback = function (callback) {
            addItemCallbacks.push(callback);
        };

        self.execAddItemCallback = function (item) {
            addItemCallbacks.forEach(function(callback) {
                callback(item);
            });
        };

    });

In the above example, there are two functions made available from the service. The insert function will allow for you to register a callback from your main function, that can be used at a later time. The exec function will allow for your data service to make use of the stored callback, passing in the new item.

1

First of all, I agree with the other comments that the decoupling seems to be a bit extreme; nonetheless, I'm sure you know more than us how much is needed for you project.

I believe an acceptable solution is to use the pub/sub (observer pattern), in which the gridSettingsService and gridDataService services do not directly update the gridMainService, but instead raise a notification that gridMainService can hook into and use to update itself.

So using the plunker that you provided, the changes I would make would be:

  1. Inject the $rootScope service into gridSettingsService, gridDataService and gridMainService.

  2. From gridSettingsService and gridDataService, stop manually injecting the gridMainServiceand instead just $broadcast the notification with the associated data:

```

// in gridDataService
$rootScope.$broadcast('GridDataItemsReady', newItems);

// in gridSettingsService
$rootScope.$broadcast('SortModelChanged', sort);

```

  1. In gridMainService, listed to the notifications and update using the passed in data:

```

// in gridMainService
$rootScope.$on('GridDataItemsReady', function(event, newItem) {
    self.gridOptions.api.addItems(newItem);
});

$rootScope.$on('SortModelChanged', function(event, newSort) {
    self.gridOptions.api.setSortModel(newSort);
});

```

And here is the updated plunker: https://plnkr.co/edit/pl8NBxU5gdqU8SupnMgy

GPicazo
  • 6,516
  • 3
  • 21
  • 24