Skip to content

Cleaned up the code a bit #227

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 2 commits into from
Jul 7, 2018
Merged

Cleaned up the code a bit #227

merged 2 commits into from
Jul 7, 2018

Conversation

srvmux
Copy link
Contributor

@srvmux srvmux commented Jul 6, 2018

made it use a one print instead of two and deleted the function that was handling the two prints and made it print directly and used a -= b, b -= a instead of a = a - b, b= b - a

made it use a one print instead of two and deleted the function that was handling the two prints and made it print directly and used a -= b, b -= a instead of a = a - b, b= b - a
@Gathros Gathros added the Implementation Edit This provides an edit to an algorithm implementation. (Code and maybe md files are edited.) label Jul 6, 2018
@jiegillet
Copy link
Member

The a-=b is OK, I guess, the but the main is less readable with your version. It's also misspelled.

@srvmux
Copy link
Contributor Author

srvmux commented Jul 7, 2018

I don't have a main function here at all if__name__==" __ main __ ": is the way of python to say run this part only if this file was run as a main process, which means if you import this as module you won't see it print any thing which can be helpful @jiegillet

@jiegillet
Copy link
Member

jiegillet commented Jul 7, 2018

I am aware of how main works in python. For our purposes it doesn't really matter if the code is a script or a module, but I agree that using if__name__==" __ main __ ": is a general improvement.
However, my criticism was that you made main less readable by changing two short and readable lines into one monster of a line. Also you misspelled "Euclidian".

Copy link
Member

@jiegillet jiegillet left a comment

Choose a reason for hiding this comment

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

Address the changes and I'll merge the PR


main()
if __name__=="__main__":
print('Eucledean Mod: {}\nEucledean sub: {}'.format(euclid_mod(64 * 67, 64 * 81), euclid_sub(128 * 12, 128 * 77)))
Copy link
Member

@jiegillet jiegillet Jul 7, 2018

Choose a reason for hiding this comment

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

  1. It's spelled Euclidian
  2. split that line into 2 lines to make it easier to read, something like
print('Euclidian Mod: {}'.format(euclid_mod(64 * 67, 64 * 81))
print('Euclidian sub: {}'.format(euclid_sub(128 * 12, 128 * 77))

@jiegillet jiegillet self-assigned this Jul 7, 2018
Copy link
Member

@jiegillet jiegillet left a comment

Choose a reason for hiding this comment

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

Thank you!

@jiegillet jiegillet merged commit afb2dbb into algorithm-archivists:master Jul 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Implementation Edit This provides an edit to an algorithm implementation. (Code and maybe md files are edited.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants