1

I have recently taken on the development of web app that has been written in AngularJS. In one of the files, myApp.js, there is a provider that is defined as follows:

.provider('myAppConf', function($routeProvider){
    var constants = {
        'HOMEPAGE': '/alarms',
        ...
    };

    // Setter function for constants
    this.setConstant = function(constant, value){
        constants[constant.toUpperCase()] = value;
    };
    ...
    // Other setter functions
    ...
    // Non- setter/ getter functions:
    this.addElementOverview = function(){
        ...
        var location = 'pages/elementbrowser';
        ...
        return '/' + location;
    }
    function addCreatePageRoute(){
        $routeProvider.when('/pages/create', {
            page: {
                editable: true,
                title: {
                    ...
                },
                layout: {
                    ...
                },
                widgets: [
                    ...
                ]
            }
        });
    }
    // More non- setter/ getter functions
    this.$get = ['$q', '$timeout', 'myAppUI' function($q, $timeout, myAppUI){
        ...
    }];
}).run(function(...){
    ...
});

On most of the pages on the site, there is a 'settings' button, which opens a dialog box that the user can use to change the settings for a given page. I have added a checkbox to that dialog box, which, when checked, I want to use to set the page on which it is checked as the 'home' page, overwriting whichever page was previously the 'home' page. If/ when the checkbox is deselected again, the home page should be set back to its original value (i.e. the /alarms page determined in the constants).

As things currently stand, I have managed to change the home page, so that it is updated to the page selected by the user- when the click 'Home' they are taken to the page on which the checkbox was selected. But as soon as they log out, their chosen home page is forgotten, and when they log in again, the default home page is their home page until they select another one.

I now want to set the user's default home page to whatever they choose as their custom home page, and am trying to use the 'setter function for constants' that is defined in the provider function above.

I have done this by calling:

myAppConf.setConstant(myAppConf.HOMEPAGE, $location.path());

when the 'Confirm' button is pressed on the dialog box (with the 'set as homepage' checkbox checked).

However, when I press the 'Confirm' button, I get a TypeError in my console which says:

TypeError: myAppConf.setConstant is not a function

I don't understand why I'm getting this error... setConstant() is defined as a function with:

this.setConstant = function(constant, value){...};

so why is the console stating that it's not a function? How can I fix this?

Edit

The function where I'm calling myAppConf.setConstant() is defined in Pages/ctrls.js as follows:

angular.module('myApp.pagse')
...
.controller('LayoutCtrl', function($scope, $route, $location, $timeout, Page, myAppConf, NotifyMgr, DialogMgr,myAppUI){
    ...
    $scope.confirm = function(e){
        ...
        if($scope.checkboxModel){
            ...
            myAppConf.setConstant(myAppConf.HOMEPAGE, $location.path());
        }else{
            console.log("homepage not changed");
        }
    };
Noble-Surfer
  • 3,052
  • 11
  • 73
  • 118

1 Answers1

1

setConstant is myAppConfProvider method, not myAppConf. If it should be available both in config and run phases, it should be defined on both a provider and an instance:

.provider('myAppConf', function(){
  ...
  var commonMethods = {
    setConstant: function (constant, value) {
      constants[constant.toUpperCase()] = value;
    },
    ...
  }

  Object.assign(this, commonMethods, {
    $get: function () {
      return commonMethods;
    }
  })
})

A cleaner way to do this is to use constant:

.constant('myAppConf', {
  _constants: { ... },
  setConstant: function (constant, value) {
    this[constant.toUpperCase()] = value;
  },
  ...
})

Since getters and setters can be considered antipattern unless they are justified, a KISS alternative is just:

.constant('myAppConf', {
  'HOMEPAGE': '/alarms',
  ...
})
Estus Flask
  • 206,104
  • 70
  • 425
  • 565
  • Thanks for your answer, but I'm not sure I follow...? `setConstant()` is currently defined in the `provider`- do I need to also define it inside the function where I'm calling it? I've edited my post to show the function I'm using to call it. – Noble-Surfer Jul 12 '17 at 09:06
  • I don't currently have a `commonMethods` variable, as you've defined... the methods are all just defined directly within the `provider`, i.e. `.provider(..., function(...){var constants = {...} "functions defined here" this.$get = [..., function(...){...}]; }).run(function(...){... });` – Noble-Surfer Jul 12 '17 at 09:36
  • commonMethods are needed if you need to access the provider in config block. Otherwise `provider` is of no use here, It could be `factory` service instead. As it was said, setter method is antipattern, so it can be just `constant` service. – Estus Flask Jul 12 '17 at 09:50
  • So rather than using `.provider('myAppConf', function($routeProvider){...}`, I should try using `.constant('myAppConf', function($routeProvider){...}` instead, as you've mentioned in your last block of code? There are other functions defined within the scope of the `provider`, i.e. not just setters & getters- does that make a difference? Maybe I'm misunderstanding, but it seems from your answer that `.constant` would only be used for setters & getters? – Noble-Surfer Jul 12 '17 at 10:03
  • `constant` presumes that it will be plain object, as it is shown in the answer. The answer takes into account only things that you've disclosed in the question. So yes, 'other functions' can make a difference. – Estus Flask Jul 12 '17 at 10:06
  • Ah OK, apologies- I didn't realise that would make a difference- I'll update my OP now to give a fuller picture of how the `provider` is structured. – Noble-Surfer Jul 12 '17 at 10:11
  • I've edited the code in my OP to show the structure of the `provider` more clearly. – Noble-Surfer Jul 12 '17 at 10:23
  • I see. Every method that is supposed to be used as `myAppConf.method` should be exposed in `$get`, not only on provider `this` (this is what `commonMethods` are for). It's still not clear why it should be `provider`. If the only reason for this is adding routes dynamically with $routeProvider, this may lead to memory leaks. – Estus Flask Jul 12 '17 at 10:30
  • I don't really know the reasoning as to why it's been written as it has- I only recently joined the company and took this project on (the author of the original code has left the company). I tried exposing the `setConstant()` function inside the `$get`, but this has given me an error when loading the page that says: `uncaught ReferenceError: setConstant is not defined`... – Noble-Surfer Jul 12 '17 at 10:53
  • There is another function call inside the `$get` to `addUserPageRoute()`, which is written no differently, and that seems to work, but `addUserPageRoute()` is defined in the `provider` with: `function addUserPageRoute(){...}`, whereas `setConstant()` is defined in the `provider` with: `this.setConstant = function(constant, value){...}` - does that make a difference to how I should call/ reference `setConstant()` in the `$get`? – Noble-Surfer Jul 12 '17 at 10:54
  • Yes, it makes a difference. It likely should be `function setConstant(){...} this.setConstant = setConstant`. Again, I see no reason why they are defined as provider methods on `this`. If they aren't used in config blocks, they shouldn't. – Estus Flask Jul 12 '17 at 11:09
  • If I do `function setConstant(constant, value){...} this.setConstant = setConstant();`, rather than `this.setConstant = function(constant, value){...}`, I get a really long error in the console that start off with: `angular.js:78 Uncaught Error: [$injector:modulerr] Failed to instantiate module eApp due to: Error: [$injector:modulerr] Failed to instantiate module fApp due to: Error: [$injector:modulerr] Failed to instantiate module myAPP due to: TypeError: Cannot read property 'toUpperCase' of undefined at setConstant `... any ideas why this is? – Noble-Surfer Jul 12 '17 at 11:57
  • 1
    `this.setConstant = setConstant`. Not `this.setConstant = setConstant()`. – Estus Flask Jul 12 '17 at 12:09
  • Ah, brilliant- that seems to work- thanks very much! – Noble-Surfer Jul 12 '17 at 12:46