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.

Sign up for GitHub

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

Jump to bottom

Refactoring #4

Closed
wants to merge 4 commits into from
Closed

Refactoring #4

wants to merge 4 commits into from

Conversation

Laguna1
Copy link
Owner

@Laguna1 Laguna1 commented Nov 25, 2020

Changing with factory functions.

@pbkabali
Copy link
Collaborator

pbkabali commented Nov 25, 2020 •
edited
Loading

Nice work replacing the constructor with a factory function. 👏 💯
I see that you have decided to put the JS into a new file main2.js. The files main1.js and logic.js are thus redundant. We can get rid of them and remove the links from the index.html.

@Laguna1
Copy link
Owner Author

Laguna1 commented Nov 25, 2020

Thank you for the review, @pbkabali

@Laguna1 Laguna1 closed this Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Laguna1 @pbkabali