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

Updated naives theorem #3

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Updated naives theorem #3

wants to merge 12 commits into from

Conversation

JDeep1234
Copy link

No description provided.

Copy link
Owner

@KTS-o7 KTS-o7 left a comment

Choose a reason for hiding this comment

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

Make the necessary changes and then PR

Copy link
Owner

Choose a reason for hiding this comment

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

The changes made to the AlphaBetaPruning/Explaination.md file are not useful for the following reasons:

  1. Redundant Code: The new implementation reintroduces the definition of the minimax function, which is already explained earlier in the document. This redundancy does not add value and clutters the explanation.

  2. Clarity and Consistency: The original document is more concise and clear in explaining the game loop and the AI's move selection process. The revised document introduces unnecessary complexity without improving the understanding.

  3. Example Dry Run: The changes to the dry run example do not provide additional insights or clarity over the original example. The original example was sufficient to demonstrate the AI's decision-making process.

  4. Documentation Quality: The original explanation was well-structured and easy to follow. The changes disrupt the flow and coherence of the document, making it harder to understand the overall logic and approach.

Overall, the changes do not enhance the documentation and may reduce its readability and effectiveness.

Copy link
Owner

Choose a reason for hiding this comment

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

The changes made in the pull request to the AlphaBetaPruning/GameOf20.py file are not useful for the following reasons:

  1. Redundant Logic: The new implementation combines multiple conditional checks into a single line, which, while concise, reduces clarity and maintainability. The original implementation had clearer separation of conditions, making it easier to understand the game logic.

  2. Function Parameters: The parameter name change from turn to is_ai_turn does not add significant value and might confuse readers familiar with the original code. Consistency in naming conventions is important for readability.

  3. Alpha-Beta Pruning: The new code introduces alpha-beta pruning logic, but the changes do not enhance the performance or accuracy of the minimax function. The original implementation already had alpha-beta pruning effectively integrated.

  4. Code Clarity: The original code had well-defined comments that explained each step of the minimax algorithm. The revised code has fewer comments, which may make it harder for others to understand the algorithm without additional context.

  5. Game Loop: The changes in the game loop, such as the handling of human and AI moves, introduce unnecessary complexity without improving functionality. The original loop was straightforward and easy to follow.

  6. Evaluation and Move Selection: The new code's approach to evaluating and selecting moves does not offer a clear advantage over the original method. The original code's approach was simpler and achieved the same goal.

In summary, while the revised code introduces some stylistic changes, it does not offer functional improvements and may reduce the overall readability and maintainability of the code.

Copy link
Owner

Choose a reason for hiding this comment

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

Useful changes but add more comments to explain

Copy link
Owner

Choose a reason for hiding this comment

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

Not a useful change

Copy link
Owner

Choose a reason for hiding this comment

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

This is done at RVCE, you could have added course code and retain RVCE

Copy link
Owner

Choose a reason for hiding this comment

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

This is fine but add more comments to code so it can be understood.

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.

2 participants