fattenap
commented
11 years ago Sounds good.
The reason I use regex is because I needed to analyse a string, but happy if you can make it work because, as you say, it would make it easier to add stuff as things change. I'd probably opt for the second approach if possible, but I'm happy for you to modify as you see fit.
On 10 October 2013 20:12, Jean-Baptiste Musso notifications@github.comwrote:
Following your comment on this commit (gulthor@a31104b
commitcomment-4294676https://github.com/gulthor/grex/commit/a31104bfa538c6ea1ebebf2068610b7380788b37#commitcomment-4294676),
I was thinking that Utils.isGraphReference() could be changed to either of the following ways:
- either dynamically generate the regex from classes definition in classes.js
- remove regex test, and test for graph references existence with something like classes.hasOwnProperty()
This would give the advantage of simply having to append stuff to classes.js without having to simultaneously update the Regex when adding more graph references in the future.
There might be a performance improvement as well, though quite minor :P.
If that's fine with you, I can try working on this minor update.
— Reply to this email directly or view it on GitHubhttps://github.com/entrendipity/grex/issues/18 .
Following your comment on this commit (https://github.com/gulthor/grex/commit/a31104bfa538c6ea1ebebf2068610b7380788b37#commitcomment-4294676), I was thinking that
Utils.isGraphReference()could be changed to either of the following ways:classes.jsclasses.hasOwnProperty()This would give the advantage of simply having to append stuff to
classes.jswithout having to simultaneously update the Regex when adding more graph references in the future.There might be a performance improvement as well, though quite minor :P.
If that's fine with you, I can try working on this minor update.