Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicate RuleEngine.deleteRule() method #13

Open
jpenna opened this issue Jul 15, 2017 · 1 comment
Open

Duplicate RuleEngine.deleteRule() method #13

jpenna opened this issue Jul 15, 2017 · 1 comment
Assignees
Labels
Milestone

Comments

@jpenna
Copy link

jpenna commented Jul 15, 2017

Hi Anand!

I was taking a look at the code and you have two method declarations for deletion.

src/engine.js

  RuleEngine.deleteRule = function (rule) {
    if (_rules[rule.group]) {
      _rules[rule.group] = _rules[rule.group].filter(function (arg) {
        return rule.group !== arg.group && rule.name !== arg.name
      })
    }
  }

  RuleEngine.deleteRule = function (name, group) {
    if (_rules[group]) {
      _rules[group] = _rules[group].filter(function (arg) {
        return group !== arg.group && name !== arg.name
      })
    }
  }

The second declaration will override the first.
In the documentation you are using the (name, group) pattern, so I was going to delete the second, but maybe you are willing to let both available, so you might consider checking if group is set or if the first passed param is an object, or whatever you think fit.

Do you want me to send a pull request for this?

@anandrajneesh anandrajneesh self-assigned this Jul 15, 2017
@anandrajneesh
Copy link
Owner

thanks @jpenna for reporting this. I guess my Java kicked in while writing this :). I will be fixing it in few days.

@anandrajneesh anandrajneesh added this to the v1.2.1 milestone Jul 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants