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

tried to fix few things #2

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

tried to fix few things #2

wants to merge 4 commits into from

Conversation

imenjd
Copy link

@imenjd imenjd commented Apr 24, 2018

hi!
I didn't really fix anything, your package is still working just as perfectly.
I used your code as a way to learn object-oriented javascript, and along the way, I found few things that your code can look better without.

imenfaith added 4 commits April 23, 2018 23:17
the filter method here does not serve any purpose.
removed the unused property grammar
the if statement is never accessed since all symbols are nonTerminals, so the condition cannot be true.
the firstSet object is already created for all Terminals and nonTerminals:

prod.derivation.forEach(function(d) {
                if (d === '') {
                    return;
                }
                else if (that.isNonTerminal(d){
                    that.firstSet[d] = {};
                    that.followSet[d] = {};
                }
                else {
                    that.firstSet[d] = {};
                    that.firstSet[d][d] = 1;
                }
            });
@BrunoRB
Copy link
Owner

BrunoRB commented May 2, 2018

Hey, thanks for contributing! unfortunately I have been really busy lately and still didn't get time to look at your changes ;( probably next week I will have some free time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants