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

P002 update.py #45

Closed
wants to merge 1 commit into from
Closed

P002 update.py #45

wants to merge 1 commit into from

Conversation

xiaowuc2
Copy link

without using function

without using function
@nayuki
Copy link
Owner

nayuki commented May 27, 2020

Problems with your code:

  • You changed my last line x, y = y, x + y for no good reason.

  • The 3 lines you added have no whitespace, in violation of my formatting standards.

  • You didn't delete my existing code. This means the new overall code will compute the solution twice and print it twice, which is very wrong.

  • You failed to notice that eulertest.py loads every solution (pXXX.py) and calls its main() function, then checks the answer against the known-good answer. This is why main() must be kept.

  • When things are written my way, main() can be declared above subfunctions and still work:

    def main():
        ans = foo()
        print(ans)
    def foo():
        bar()
    def bar():
        pass
    

    When things are written your way, all subfunctions and global variables/constants must be declared above main():

    def foo():
        bar()
    def bar():
        pass
    ans = foo()
    print(ans)
    
  • Why are you even proposing this change? It is not an improvement on my work. It is a disimprovement.

@nayuki nayuki closed this May 27, 2020
@xiaowuc2
Copy link
Author

I know I've done dis-improvement in your work and there are time complexity I know those things I made that just for beginners that they can understand it when they are not using ' , ' then they can't write, x = y
y=x
just to make fundamentals clear I did that, and I am a beginner as well so I thought it would help others while checking out.

and the second problem you said I had no idea about that, I'm begging pardon for that.

@jeremydouglass
Copy link

jeremydouglass commented May 27, 2020

@nayuki you might want to consider just warning people when they open pull requests that PRs will not be accepted. This type of repo tends to attract unsolicited contributions from enthusiastic beginners by its nature -- but from your responses it seems that perhaps PRs make you... angry? Maybe just drop in a PR template that says "Don't PR!" and it will appear before they try, saving their time and energy as well protecting your own.

https://help.github.com/en/github/building-a-strong-community/creating-a-pull-request-template-for-your-repository

Another complimentary strategy is to leave a single open PR in your Pull Requests lists, called eg. DO NOT PULL REQUEST. Then anyone clicking on the PR tab will see it.

@nayuki
Copy link
Owner

nayuki commented May 27, 2020

leave a single open PR in your Pull Requests lists, called eg. DO NOT PULL REQUEST. Then anyone clicking on the PR tab will see it

@jeremydouglass Done, thank you for your kind and helpful advice.

Created PR #46: Do not open pull requests. I will reject them.

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