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

General implementation cleanup #849

Merged

Conversation

stormofice
Copy link
Contributor

@stormofice stormofice commented Aug 27, 2021

This PR combines some easy syntactic fixes to standardize implementations for automatic testing and validation (in reference to #824 and #691).

These changes were done (see commit messages for more details):

  • Huffman Encoding [java]: Move Huffman class to the top of the file
  • Tree Traversal [java]: Move main method into Tree class
  • Verlet Integration [java]: Move VerletValue class into Verlet class
  • Monte Carlo [cpp]: Hardcode sample count (taken from julia implementation)

I tried to keep the formatting close to the original implementations.

This removes the need for user input
Executing a single java file without compilation (in java >= 11) only works if the first
class it finds contains the main method.
@ntindle
Copy link
Member

ntindle commented Aug 28, 2021

[lang: java] [lang: cpp]

@ntindle
Copy link
Member

ntindle commented Aug 28, 2021

lol whoops though #847 was merged already

@ntindle
Copy link
Member

ntindle commented Aug 28, 2021

[lang: java] [lang: cpp]

@github-actions github-actions bot added the lang: java Java programming language label Aug 28, 2021
@ntindle
Copy link
Member

ntindle commented Aug 28, 2021

[lang: cpp]

@ntindle
Copy link
Member

ntindle commented Aug 28, 2021

[lang: c++]

@github-actions github-actions bot added the lang: c++ C++ programming language label Aug 28, 2021
Copy link
Member

@Amaras Amaras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks all good to me.
I'd merge right now, but I'd like to get a second opinion.

@Trashtalk217 Trashtalk217 self-assigned this Aug 30, 2021
@leios
Copy link
Member

leios commented Aug 31, 2021

This looks good, but did the line numbers for the Monte Carlo and Tree Traversal chapters change? If so, the in-text code-blocks should be updated. It looks like it was done for Verlet and it doesn't seem to be necessary for Huffman. I was just double-checking before merging.

@stormofice
Copy link
Contributor Author

The fixes for Tree Traversal and Monte Carlo did not require inserting code at the start of the file.
As the quoted functions in these chapters are at the beginning of the file, nothing needs to be changed as no code blocks got shifted downwards by inserting code above them.

However this is different for the other two implementations, which is why the numbers needed to be adjusted there but not for Tree Traversal / Monte Carlo.

[When the full code exmaple should be shown (like it is the case at the end of chapters), no line numbers are required in the md file and nothing needs to change there as well]

@leios
Copy link
Member

leios commented Aug 31, 2021

Right, great! Thanks again. Happy to merge!

@leios leios merged commit cbe4b61 into algorithm-archivists:master Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang: c++ C++ programming language lang: cpp lang: java Java programming language
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants