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

factorial #40

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

factorial #40

wants to merge 1 commit into from

Conversation

yegor256
Copy link
Member

@yegor256 yegor256 commented Apr 5, 2024

Let's calculate factorial


PR-Codex overview

The focus of this PR is to refactor the code to replace the A class with Factorial class to calculate factorial efficiently.

Detailed summary

  • Renamed A class to Factorial class
  • Updated method logic to calculate factorial efficiently
  • Changed method invocation from new A().get() to new Factorial().get()

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

}
return new A(d - 1).get();
return new Factorial(d - 1).get() * d;
Copy link

Choose a reason for hiding this comment

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

Using a static method would be better here because creating an object with each recursive call fills up the stack, reducing efficiency.

Copy link
Member Author

Choose a reason for hiding this comment

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

@DIY0R this is the point of the optimization there -- to replace object with static method automatically

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.

None yet

2 participants