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

Feature/refactor #3

Closed
wants to merge 2 commits into from
Closed

Conversation

antony
Copy link

@antony antony commented Mar 6, 2017

I've separated out the core components of the project for readability and understandability.

  • A p2p module
  • an http server
  • The blockchain

and also separated out their individual components.

I've also refactored the code a bit for readability, such as removing nested ifs, and nested functions, and introduced linting in order to spot errors and possible bugs.

I'd probably like to start retrofitting tests if this PR gets approved.

Cheers,
Antony

@antony antony mentioned this pull request Mar 6, 2017
@p0o
Copy link

p0o commented Mar 16, 2017

I actually don't like this approach because the original code is written with a clean Functional Programming approach for learning purposes. If you wanted to improve the previous code you should have embraced the same approach with Higher Order Functions, currying, composing, etc. Putting everything inside classes like a Java code doesn't make it necessarily better.

@lhartikk
Copy link
Owner

As I described in more detail in #2 , I intend to leave the main version of naivechain as a single 200 lines of code file.

@lhartikk lhartikk closed this Mar 16, 2017
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.

3 participants