Skip to content

Solution#897

Open
Sacchar20 wants to merge 2 commits into
mate-academy:masterfrom
Sacchar20:develop
Open

Solution#897
Sacchar20 wants to merge 2 commits into
mate-academy:masterfrom
Sacchar20:develop

Conversation

@Sacchar20

Copy link
Copy Markdown
image

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

I am requesting changes because there are still two critical mismatches with the explicit task requirements. First, the specification requires a dedicated helper sha256_hash_str(to_hash: str) -> str and that this be used for hashing in the brute-force logic, but in app/main.py you currently call sha256(...).hexdigest() directly inside check_range, which violates [CHECKLIST ITEM #8]/#9. To fix this, define sha256_hash_str at the top level (e.g., near your constants) and replace the direct sha256 call in check_range with a call to this helper.

Second, after aggregating worker results, you print whatever was found without verifying that all 10 hashes in PASSWORDS_TO_BRUTE_FORCE have corresponding passwords, which does not fully satisfy the requirement to find and print all 10 passwords [CHECKLIST ITEM #1, #3, #4]. At the end of main, add a validation step such as checking len(final_results) == 10 and then iterating over PASSWORDS_TO_BRUTE_FORCE to print each hash and its password, so any missing match is immediately obvious (and can raise or log an error). A minor improvement (not blocking) would be to ensure the output order matches PASSWORDS_TO_BRUTE_FORCE for clarity. Overall, your brute-force and multiprocessing logic are solid; addressing these points will bring the solution fully in line with the stated requirements.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

Footnotes

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread app/main.py Outdated

for i in range(start, end):
pwd = f"{i:08}"
current_hash = sha256(pwd.encode("utf-8")).hexdigest()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The requirements specify using a dedicated sha256_hash_str(to_hash: str) -> str helper instead of calling sha256(...).hexdigest() directly. Consider defining that function and using it inside check_range to comply with checklist item #8/#9.

Comment thread app/main.py Outdated
Comment on lines +51 to +56
final_results = {}
for local_dict in results:
final_results.update(local_dict)

print("\nBrute-force results:")
for h, p in final_results.items():

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

After aggregating results, you print whatever was found, but the task requires that all 10 passwords be found and printed. It would be safer to verify that final_results covers all entries in PASSWORDS_TO_BRUTE_FORCE (e.g., length == 10 and/or iterating over PASSWORDS_TO_BRUTE_FORCE for output) to satisfy checklist items #1, #3, and #4.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

Your solution is approved because it fully meets the functional and technical requirements for this task. The brute-force logic correctly covers the entire 8-digit numeric space (0 to 99,999,999) and uses zero-padded strings, ensuring all passwords are exactly 8 digits and numeric-only. You properly implemented the required sha256_hash_str(to_hash: str) -> str helper and consistently use it in the brute-force loop, satisfying the hashing constraint and avoiding direct hashlib.sha256 calls in the core search logic. The code finds all 10 passwords, validates that all corresponding hashes have been discovered before proceeding, and then prints the passwords in the exact order of the original hash list, which aligns with the task’s expectations.

The use of multiprocessing.Pool with cpu_count() is an effective and appropriate optimization for distributing the search workload across processes. One minor improvement (not required for this task) is that logging or print calls inside worker processes may interleave, so if you ever need cleaner logs, you could centralize output in the main process. Overall, you’ve implemented the requirements accurately and robustly, and the remaining checklist item about including a console output screenshot is documentation-related rather than a code blocker.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

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