Skip to content

Euclidean algorithm: Output standardization #877

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

Merged
merged 32 commits into from
Jan 13, 2023

Conversation

stormofice
Copy link
Contributor

@stormofice stormofice commented Oct 12, 2021

The following implementations were not fully standardized:

Implementations which seem to contain errors or do not run:

  • clojure
  • factor (can't get to run either online or locally)
  • smalltalk (apparently this is written in the Pharo dialect so smalltalk (as per the original PR Euclidean algorithm for Smalltalk. #454), but it still does not work)
  • viml (implementation seems kind of wrong, at least when I ran it online)

No need to and/or not standardizable:

  • emojicode (esolang)
  • lolcode (esolang)
  • matlab (output misses the newline after [#], but as there are no plans in automatically validating it, I think this can be ignored)
  • piet (graphical language)
  • scratch (mostly graphical language)
  • whitespace (esolang)

Two implementations had bugs (fixed now):

  • Javascript
  • Scala

@Amaras Amaras added Hacktoberfest The label for all Hacktoberfest related things! hacktoberfest-accepted Implementation Edit This provides an edit to an algorithm implementation. (Code and maybe md files are edited.) Standardisation This implies changes in most of the algorithms's implementations and removed hacktoberfest-accepted labels Oct 16, 2021
@leios
Copy link
Member

leios commented Oct 26, 2021

I know this is still a draft, but is it close to being reviewed?

@stormofice
Copy link
Contributor Author

The languages which I standardized already can be reviewed, however it would be good to gather some more implementations before the final review.

@stormofice
Copy link
Contributor Author

I am done with all the implementations which can be standardized properly (excluding esoteric and graphical languages).

Clojure / Factor / Smalltalk / viml were not standardized, as I could not figure out how to run them. I suspect that they either contain syntactic/semantic errors or that they do not run/build with newer versions of their respective programming language.

Apart from these outliers everything should be ready for review.

Currently there do not seem to be any reviewers active, which could take a look at the broken implementations.

However I really do want to stay back from merging this until those aforementioned implementations are fixed or removed, as just leaving them out would be against the original intent of this standardization work.

@stormofice stormofice marked this pull request as ready for review December 1, 2021 02:54
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.

I will approve this, but it does need to be merged up to master. There were a few small changes to the C and scheme code, but they are easy fixes

@@ -3,8 +3,8 @@ object Euclid {
def euclid_sub(a: Int, b: Int): Int =
(Math.abs(a), Math.abs(b)) match {
case (0, _) | (_, 0) => 0
case (x, y) if x < y => euclid(x, y - x)
case (x, y) if x > y => euclid(x - y, y)
case (x, y) if x < y => euclid_sub(x, y - x)
Copy link
Member

Choose a reason for hiding this comment

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

Wait, how did this code run before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It didn't.
The scala and javascript implementation needed some (small) fixes to work.


WRITE(*,'(a)') '[#]'
WRITE(*,'(a)') 'Subtraction-based euclidean algorithm result::'
WRITE(*, '(g0)') euclid_sub(a, b)
Copy link
Member

Choose a reason for hiding this comment

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

Did we just not run the fortran code before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We ran the two methods the other way around

@Amaras
Copy link
Member

Amaras commented Jan 13, 2023

Since implementation and review of the missing languages don't seem to be planned any time soon, I'll just merge this as long as everything compiles correctly.

@Amaras Amaras merged commit 6c905ec into algorithm-archivists:main Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hacktoberfest The label for all Hacktoberfest related things! Implementation Edit This provides an edit to an algorithm implementation. (Code and maybe md files are edited.) Standardisation This implies changes in most of the algorithms's implementations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants