Solution for mirror words challenge #38

Aseel-AbuKmail · 2025-01-05T09:26:21Z

Name: Mirror Words Challenge
About: A template PR for code review with a checklist

Behavior

Files

The file name describes the function's behavior
There is a module docstring in the function file
The test file's name matches the function file name -
/tests/test_file_name.py
There is a module docstring in the tests file

Unit Tests

The test class has a helpful name in PascalCase
The test class has a docstring
Every unit test has
- A helpful name
- A clear docstring
- Only one assertion
- There is no logic in the unit test
All tests pass
There are tests for defensive assertions
There are tests for boundary cases

Function Docstring

The function's behavior is described
The function's arguments are described:
- Type
- Purpose
- Other assumptions (eg. if it's a number, what's the expected range?)
The return value is described
- Type
- Other assumptions are documented
The defensive assertions are documented using Raises:
- Each assumption about an argument is checked with an assertion
- Each assertion checks for only one assumption about the argument
Include 3 or more (passing!) doctests

The Function

The function's name describes it's behavior
The function's name matches the file name
The function has correct type annotations
The function is not called in the function file

Strategy

Do's

Variable names help to understand the strategy
Any comments are clear and describe the strategy
Lines of code are spaced to help show different stages of the strategy

Don'ts

The function's strategy is not described in the documentation
Comments explain the strategy, not the implementation
The function does not have more comments than code
- If it does, consider finding a new strategy or a simpler implementation

Implementation

Create Solution for mirror words challenge

abdoalsir · 2025-01-05T11:41:03Z

Hi Aseel,
I hope you are having a great day.
I see that your solutions didn't pass some checks, if you need assistance feel free to reach out

Aseel-AbuKmail · 2025-01-05T16:13:13Z

Hi Aseel, I hope you are having a great day. I see that your solutions didn't pass some checks, if you need assistance feel free to reach out

Hello, Abdulrahman!
Thank you for offering to help; I really appreciate it!

abdoalsir

Overall Feedback:

The function and test files are well-constructed and fulfill many criteria for clear and maintainable code. You have effectively utilized docstrings and tests, and the function successfully passes all provided checks. Nevertheless, there are several areas where enhancements could be made to increase the robustness and user-friendliness of the code:

Function Naming: The name of the function could be more descriptive to eliminate any ambiguity regarding its purpose. Aligning it with the file name (e.g., mirror_words) may clarify its functionality.
Type Annotations: Incorporating type annotations for both the input argument and the return value would enhance clarity for readers who may not refer to the docstring.
Docstring Enhancements: Although your docstring is clear, including information about the treatment of special characters and numbers (as indicated in your tests) along with a high-level overview of the strategy would improve its thoroughness and usefulness for others who may review or extend your code.
Doctests: Adding a third doctest that illustrates the handling of special characters would further enrich the documentation of the function.
Code Structure: Enhancing the spacing and organization of the code will contribute to making the implementation more readable and accessible to a wider audience.
Commented Lines: If the commented-out import statements are unnecessary, consider removing them. If they serve a purpose, please add a clarifying comment to explain their intent.

Overall, excellent work!😇 Implementing these recommendations will raise the quality of your code from great to outstanding.

abdoalsir · 2025-01-07T20:58:06Z

solutions/mirror_words_challenge.py

+
+    Returns:
+        str: A new string where each word is reversed, but the word order and punctuation
+             remain unchanged.


Your docstring very clear and easy to read, I have a small comment here on the return value of the function. You mentioned the type of the return value and you mentioned punctuation. You can also include that numbers and special characters remain unreversed as you mentioned in the test file. This will add more clarity for some just checking the function file.

Thank you for pointing that out! I'll update the return value description to include that numbers and special characters remain unreversed for added clarity.

abdoalsir · 2025-01-07T21:00:20Z

solutions/mirror_words_challenge.py

+
+        >>> reverse_words("Python is fun")
+        'nohtyP si nuf'
+


The doctests are very clear and shows exactly what the output will be. Consider adding a 3rd doctest and maybe include special characters to show they remain unreversed.

Thank you for the feedback! I'll add a third doctest with special characters to demonstrate how they remain unreversed.

abdoalsir · 2025-01-07T21:03:20Z

solutions/mirror_words_challenge.py

+
+
+def reverse_words(sentence):
+    """


The function name describes what the function do, but it can be a little confusing since someone might understand it reverses the entire sentence not just the words. I believe it would be better to use the same file name mirror_words. It gives a better description of the functions behavior.

Moreover, make sure to include the type annotation, you mentioned in the function docstring the argument and the return value of the function, it would be more clear for someone who just reads the functions name to understand the argument and return value types. The function name would look a lot better like this:

def mirror_words(sentence: str) -> str

Thank you for the feedback! I will rename the function to mirror_words for clarity and update the function signature to include type annotations as suggested.

abdoalsir · 2025-01-07T21:10:42Z

solutions/mirror_words_challenge.py

+    if not isinstance(sentence, str):
+        raise TypeError("Input must be a string")
+
+    words = sentence.split()


Your code is readable, but there is room for improvement in spacing and structure to make it clearer and more organized, so it makes it easier for anyone even those with no experience can read it and understand every stage of the functionality.

Thank you for the suggestion! I will adjust the spacing and structure to make the code more organized and readable for everyone.

abdoalsir · 2025-01-07T21:16:07Z

solutions/tests/test_mirror_words_challenge.py

+sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), "../../")))
+
+# from mirror_words_challenge import reverse_words
+# from solutions.mirror_words_challenge import reverse_words


If these lines are unnecessary, delete them. However, if they are intentionally retained for reference or debugging purposes, add a clarifying comment explaining why they are there.

When I push the code, I face an issue where the mirror_words_challenge file cannot be found in the solutions directory. Thank you for the feedback! I'll review these lines to determine if they are necessary. If they are not, I will remove them. If they are intentional for debugging or reference, I will add a comment to clarify their purpose.

abdoalsir · 2025-01-07T21:18:44Z

solutions/mirror_words_challenge.py

+    """
+    Reverses each word in a given sentence while maintaining the order of the words
+    and correctly handling punctuation marks at the end of words.
+


The docstring is well written and it can be even better with a strategy to make the code even easier to understand for others who might work with or review it in the future.

Thank you for the feedback! I'll enhance the docstring by including an example and a brief note on the function's behavior to make it even clearer for others.

abdoalsir · 2025-01-07T21:28:13Z

solutions/tests/test_mirror_words_challenge.py

@@ -0,0 +1,86 @@
+# test_mirror_words_challenge


The file name already shows what file it's, so I think it would be better to remove this comment so the code appears more clean and professional.

Got it, I will delete the comment as suggested. Thank you!

abdoalsir · 2025-01-07T21:28:24Z

solutions/mirror_words_challenge.py

@@ -0,0 +1,44 @@
+# mirror_words_challenge


The file name already shows what file it's, so I think it would be better to remove this comment so the code appears more clean and professional

Ok, I will remove the comment as you suggested. Thank you for the feedback!

…al characters

… function's behavior

…ath line

…search path

Aseel-AbuKmail · 2025-01-10T07:27:49Z

@abdoalsir
Hi, I’ve implemented your suggested feedback. If you don’t have any further suggestions, please go ahead and merge it into the main branch and delete my challenge branch.

Aseel-AbuKmail · 2025-01-10T17:06:34Z

Lessons Learned from My Mistakes

How to Resolve ET CI Check Issues:

1. For py_formatting:
Run the following commands in the terminal:

pip install ruff
ruff format
ruff check

OR:

Run the following commands in the terminal:

pip install black
black --check <file_location>
black <file_location>

2. For py_tests

Verify that every file is in the correct path.

Solution for mirror words challenge

289863d

Create Solution for mirror words challenge

Aseel-AbuKmail self-assigned this Jan 5, 2025

Aseel-AbuKmail added the challenge a new challenge label Jan 5, 2025

Aseel-AbuKmail linked an issue Jan 5, 2025 that may be closed by this pull request

Mirror Words Challenge #37

Closed

Aseel-AbuKmail added 5 commits January 5, 2025 12:21

Update Solution for mirror words challenge

63ceb0e

Create mirror_words_challenge.py

d11d3ce

Create test_mirror_words_challenge

16dafa2

Rename test_mirror_words_challenge to test_mirror_words_challenge.py

feff3c7

Delete solutions/Solution for mirror words challenge

99285bb

Aseel-AbuKmail added 9 commits January 5, 2025 16:17

Final update mirror_words_challenge.py

c080bc6

Final update test_mirror_words_challenge.py

1c8a18e

Last Update mirror_words_challenge.py

f2e63da

Last Update test_mirror_words_challenge.py

ba48437

Update test_mirror_words_challenge.py

14cd949

Update test_mirror_words_challenge.py

cd6f6d9

Update mirror_words_challenge.py

7056d7e

Update mirror_words_challenge.py

d3436fb

Update test_mirror_words_challenge.py

77e12b2

abdoalsir self-requested a review January 6, 2025 14:16

abdoalsir added this to the Milestone: Code Reviews and Improvements milestone Jan 7, 2025

abdoalsir reviewed Jan 7, 2025

View reviewed changes

Aseel-AbuKmail added 7 commits January 8, 2025 08:43

Delete the mirror_words_challenge comment in the first line

f6b637b

Changing reverse_words to mirror_words

b2d9bc2

Adding a third doctest with special characters to demonstrate

ba23abc

update the return value description to include that numbers and speci…

ab25c51

…al characters

Delete test_mirror_words_challenge comment in the first line

2808c2b

Adjust the spacing and structure

6ad72b4

Enhance the docstring by including an example and a brief note on the…

9886c59

… function's behavior

Aseel-AbuKmail added 11 commits January 8, 2025 10:20

Replace reverse_words with mirror_words and remove the complex file p…

5aded70

…ath line

Add the directory containing mirror_words_challenge.py to the module …

4c3c4d9

…search path

Finish def with :

744bf9a

Update mirror_words_challenge.py

355fd0f

Update test_mirror_words_challenge.py

28bb43b

Update test_mirror_words_challenge.py

47cd223

Final Update mirror_words_challenge.py

ab7ab3e

Final Update test_mirror_words_challenge.py

e9df098

Update the lines path for test code

d7bb41f

Last Update mirror_words_challenge.py according to the group 15!!

13bba23

Last Update test_mirror_words_challenge.py according to the group 15

a9b8260

abdoalsir merged commit bdb706f into main Jan 11, 2025
10 checks passed

abdoalsir deleted the Challenge/1-Mirror_Words-Aseel-AbuKmail-branch branch January 11, 2025 18:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Solution for mirror words challenge #38

Solution for mirror words challenge #38

Aseel-AbuKmail commented Jan 5, 2025 •

edited by abdoalsir

Loading

abdoalsir commented Jan 5, 2025

Aseel-AbuKmail commented Jan 5, 2025

abdoalsir left a comment

abdoalsir Jan 7, 2025

Aseel-AbuKmail Jan 8, 2025

abdoalsir Jan 7, 2025

Aseel-AbuKmail Jan 8, 2025

abdoalsir Jan 7, 2025

Aseel-AbuKmail Jan 8, 2025

abdoalsir Jan 7, 2025

Aseel-AbuKmail Jan 8, 2025

abdoalsir Jan 7, 2025

Aseel-AbuKmail Jan 8, 2025

abdoalsir Jan 7, 2025

Aseel-AbuKmail Jan 8, 2025

abdoalsir Jan 7, 2025

Aseel-AbuKmail Jan 8, 2025

abdoalsir Jan 7, 2025

Aseel-AbuKmail Jan 8, 2025

Aseel-AbuKmail commented Jan 10, 2025

Aseel-AbuKmail commented Jan 10, 2025 •

edited

Loading

Solution for mirror words challenge #38

Solution for mirror words challenge #38

Conversation

Aseel-AbuKmail commented Jan 5, 2025 • edited by abdoalsir Loading

Name: Mirror Words Challenge About: A template PR for code review with a checklist

Behavior

Files

Unit Tests

Function Docstring

The Function

Strategy

Do's

Don'ts

Implementation

abdoalsir commented Jan 5, 2025

Aseel-AbuKmail commented Jan 5, 2025

abdoalsir left a comment

Choose a reason for hiding this comment

Overall Feedback:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aseel-AbuKmail commented Jan 10, 2025

Aseel-AbuKmail commented Jan 10, 2025 • edited Loading

Aseel-AbuKmail commented Jan 5, 2025 •

edited by abdoalsir

Loading

Name: Mirror Words Challenge
About: A template PR for code review with a checklist

Aseel-AbuKmail commented Jan 10, 2025 •

edited

Loading