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

Tree Traversal: Output Standardization #857

Merged
merged 92 commits into from
Oct 25, 2021

Conversation

lazyprop
Copy link
Contributor

@lazyprop lazyprop commented Sep 3, 2021

This is to standardize language implementations for the tree traversal chapter. It is based on #855.

@lazyprop
Copy link
Contributor Author

lazyprop commented Sep 3, 2021

I have made changes to many of the implementations, but there are a few that are
problematic.

  • The PHP implementations has a weird way of printing node data.
  • The Haskell implementation numbers the nodes in an entire different way from
    the others.
  • The Coconut implementation works slightly differently. Shouldn't be hard to
    fix it though.
  • OCaml is missing postorder and preorder DFS. Unfortunately I don't know OCaml so I can't write those functions myself.

I'll update this list tomorrow as I'm way too sleepy right now.

@Amaras Amaras added Implementation Edit This provides an edit to an algorithm implementation. (Code and maybe md files are edited.) lang: c C programming language lang: c++ C++ programming language lang: javascript Javascript programming language lang: julia Julia programming language lang: python Python programming language lang: rust Rust programming language labels Sep 3, 2021
@Amaras
Copy link
Member

Amaras commented Sep 3, 2021

Oh well, apparently I have write access to your repo.
The Coconut output should be okay (it was simply a matter of adding [#] everywhere)

@Amaras Amaras added the lang: coconut Coconut programming language label Sep 3, 2021
@lazyprop
Copy link
Contributor Author

lazyprop commented Sep 3, 2021

@Amaras Coconut does not have any nodes containing 0 which every other language does. This can be fixed just by changing the condition in create_tree to this:

    if num_rows == 0:
        return Node(0, ())

@leios
Copy link
Member

leios commented Sep 4, 2021

Great work with this, maybe we hide Ocaml for now, as it is missing some code snippets?

I think @jiegillet might be willing to fix the haskell?

I don't know about PhP, though. Does it need a rework entirely?

stormofice and others added 2 commits September 4, 2021 11:42
* Standardize julia output

* Standardize kotlin output

This also fixes a previous bug, which caused the time and velocity
values to not get printed correctly.

* Standardized c output

* Standardized cpp output

* Standardized lisp output

* Standardized fortran output

I was not able to prevent the preceding whitespaces, but they can
just be trimmed.

* Standardized go output

* Standardized java output

* Standardize javascript output

* Standardize nim output

* Standardize python output

* Standardize ruby output

As the original implementation only returned the time and not
the velocity, the code needed to be adjusted a bit. Now it returns
the two values as an array which gets deconstructed and printed.

* Standardize rust output

* Standardize swift output

* Standardized haskell output

* Standardized haskell output (no quote marks)

* attempt at fix for asm

Co-authored-by: Jérémie Gillet <jie.gillet@gmail.com>
Co-authored-by: James Schloss <jrs.schloss@gmail.com>
@lazyprop
Copy link
Contributor Author

lazyprop commented Sep 4, 2021

Yeah hiding Ocaml would be good for now.

I don't know Haskell but I think it can be fixed by just creating a createTree procedure which numbers nodes appropriately.

As for PHP, I think I have a fix. The only thing that needed changing was the generate_tree function (that seems to be a common problem in the implementations). I also removed an if condition from DFSRecursive so maybe the line numbers need changing. I have zero experience with PHP though so if someone can review the PHP changes that would be nice.

@jiegillet
Copy link
Member

I think @jiegillet might be willing to fix the haskell?

Of course, please give me a bit of time.

Yeah sorry, back in the day I used to do things my way, since Haskell programs are structured quite differently anyway... I'll fall into the ranks now.

@Amaras
Copy link
Member

Amaras commented Oct 23, 2021

Okay, I had to merge, so it might have caused problems with the line numbers, we'll have to make sure we check them before merging.

I did that job because @lazyprop wanted someone to update the PR correctly. Hopefully the history is not too messy (I didn't want to force-push when I was not sure of myself)

@leios
Copy link
Member

leios commented Oct 24, 2021

I think the Java file still needs to be updated (because of the merge), but I think that's the only thing to change and we can then squash and merge this guy.

I think the squash will get rid of the messy history.

@lazyprop
Copy link
Contributor Author

I think the Java file still needs to be updated

@leios sorry I don't understand what exactly needs to be updated. It seems to be exactly what it should be.

@leios
Copy link
Member

leios commented Oct 25, 2021

I think there are 2 Java files on this branch, then? Tree.java and MainClass.java we recently changed to use Tree.java instead of MainClass, so the [#]\n statements need to be added to Tree.java and MainClass.java needs to be removed.

@lazyprop
Copy link
Contributor Author

I have removed MainClass.java updated Tree.java with the [#]\n statements.

@leios
Copy link
Member

leios commented Oct 25, 2021

I think we are good to squash and merge this guy, but I wanted to mention that there seem to be 4 languages that are not up-to-date:

  • matlab
  • smalltalk
  • scratch
  • emojicode

I think scratch can be scratched off the list, but the other 3 might be standardized in the future? No matter the case, this is a good step and I am happy to merge.

Copy link
Member

@leios leios left a comment

Choose a reason for hiding this comment

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

The commit history is not clean, but we'll do a squash and merge

@leios leios merged commit 2000ab3 into algorithm-archivists:master Oct 25, 2021
@lazyprop
Copy link
Contributor Author

Thanks for merging. I think Smalltalk was added two days ago here (#453). Maybe we can get Neverik to fix the output format?

As for Matlab, I don't have a license so no way to test the code. Maybe someone who has a license can do it.
MATLAB is Evil

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Standardisation This implies changes in most of the algorithms's implementations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet