0

I have a method that is emitted from a child. The method is as follows to enable a single notification:

handleOne(name, enabled) {
  if (!this.hasPerms()) {
    return;
  }
  if (!!this.loggedInUser) {
    this.toggleNotif({
      notifName: name,
      newAnalytics: {
        type: 'brand',
        true,
      },
    });
  } else {
    login.redir('new');
  }
},

I want to create a new method that handles enabling all notifications. They require separate actions internally (toggleNotif and toggleAllNotif) but I also had these handlers separated. Here was the new method I created:

handleAll() {
  if (!this.hasPerms()) {
    return;
  }
  if (!!this.loggedInUser) {
    this.toggleAllNotif({
      newAnalytics: {
        type: 'allBrand',
        true,
      },
    });
  } else {
    login.redir('new');
  }
},

they are sort of similar except the new one does not take arguments and the internal method calling the action is different. Would it make sense to refactor the handler into one method and made the params conditional or is that just messy and overthinking things? Kind of like this:

handleAll(turnOnAll=false, name=false, enabled=false) {
  if (!this.hasPerms()) {
    return;
  }
  if (!!this.loggedInUser) {
    if (turnOnAll) {
      this.toggleAllNotif({
        newAnalytics: {
          type: 'allBrand',
          true,
        },
      });
    } else { 
      this.toggleNotif({
        notifName: name,
        newAnalytics: {
          type: 'brand',
          true,
        },
      });
    } 
  } else {
    login.redir('new');
  }
},

1 Answers1

0

The only thing that changes is the name, the toggleNotif, and the type, so use arguments to handle those cases:

handle(method, type, name) {
   if (!this.hasPerms()) {
      return;
   }
   if (!!this.loggedInUser) {
      this[method]({
         notifName: name,
         newAnalytics: {
            type,
            true,
         },
      });
   } else {
      login.redir('new');
   }
},

Then call, eg, handle('toggleNotif', 'brand', someName) or handle('toggleAllNotif', 'allBrand').

CertainPerformance
  • 356,069
  • 52
  • 309
  • 320
  • thanks....`handleAll` doesn't have the `notifName` in in the inner function call. That is only for `handleOne` because it needs to know the `notifName` (notification name) it should be enabling – MarioLeoma Apr 19 '21 at 22:40
  • Yep, the value will be `undefined` - should be fine. You can [conditionally add it](https://stackoverflow.com/q/11704267) to the object if the very existence of the property causes problems, but it's very unlikely to be required. – CertainPerformance Apr 19 '21 at 22:41