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

Refactored and Optimised Code base #1699

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rahulgurujala
Copy link

Optimized all code bases and refactored code.

Copy link
Contributor

Choose a reason for hiding this comment

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

sum(d_cards) is not evaluated. It should be evaluated and
Sum of both d_cards & p_cards should equal to 21

Copy link
Contributor

Choose a reason for hiding this comment

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

yes.

Copy link
Contributor

Choose a reason for hiding this comment

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

  • Removing comments not a good idea as its not enhancing the code in any way unless
  • One way to factor this block of code is to assign input_str[-2] to a variable, this way its more readable hence the comments are no longer needed 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

yes.

Copy link
Contributor

Choose a reason for hiding this comment

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

match-case is faster than if-else. If you could do that instead of the if-else

match choice: case "1": ... case _: ...

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, @OfficialAhmed is right.
Match case are being introduced in not but recent version of python.
They are somewhat new in to this language.

Comment on lines 52 to 53
else:
continue
Copy link
Contributor

Choose a reason for hiding this comment

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

can be deleted

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.

4 participants