Capital letters

[Kareiman]

---

name: solution review
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

• The code passes the formatting checks
• The code passes all Ruff linting checks
• The code has no (reasonable) Pylint errors
  • In code review, you can decide when fixing a Pylint error is helpful and
    when it's too restricting.
• Variables are named with snake_case
• Variable names are clear and helpful
• The code follows the strategy as simply as possible
• The implementation is as simple as possible given the strategy
• There are no commented lines of code
• There are no print statements anywhere
• The code includes defensive assertions
• Defensive assertions include as little logic as possible

[Frank2446-dotcom]

The challenge is tackled effectively, with a well-implemented solution and comprehensive testing to ensure functionality across a variety of scenarios, including standard, edge, and defensive cases. Excellent work on maintaining clarity, input validation, and robust test coverage, demonstrating a thorough understanding of the task—well done!

[Emanfalouji]

just correct that uppercase is a one word

[Emanfalouji]

it could be corrected to this format
user_text (type): description
like
user_text (str): The user input text to be converted to uppercase.

[Emanfalouji]

you could add the return type here to be like this
(str) : user_text in capital letters

[Emanfalouji]

could you add as you mentioned in the assrtionError message to be like this
AssertionError: If the input is empty or contains only spaces.

[Emanfalouji]

could you rewrite Examples with capital E
also the previous lines , to start with capital letter (Raises , Returns and Parameters)

[Emanfalouji]

could you add here the Test categories with Standard cases, Edge cases, and Defensive tests

[Kareiman]

done

[Frank2446-dotcom]

1. Current: convert_to_capital

Improvement: While this name is clear, it could be more explicit in describing what the function does, such as convert_to_uppercase, to clearly convey that it deals with uppercase conversion. "Capital" can be ambiguous because people might think it deals with capitalization rules (e.g., first letter capitalized), not case swapping.

Suggested Improvement: convert_to_uppercase

[Kareiman]

done.

[Frank2446-dotcom]

Improvement: The module docstring does not specify the function’s behavior as clearly as it could. Consider improving the explanation of the function and its purpose.

"""
This function takes a user input string and returns the string with all characters in uppercase.
It handles edge cases where the input is empty or contains only spaces.
"""

[Kareiman]

I added more description but explaining the edge behavior is mentioned within the tests

[Frank2446-dotcom]

Improvement: The use of strip() and then checking if not user_text is good. However, you can directly handle empty strings in the assertion, which makes the function simpler:
Action: Use strip() to handle spaces at the beginning or end of the input more cleanly in the assertion check.

e.g

if not user_text.strip():
raise AssertionError("Entry cannot be empty or just spaces")

[Kareiman]

I already call strip() when checking for empty strings, so there’s no need to call it again on user_text, it is redundant.

[Frank2446-dotcom]

The test file’s docstring is minimal and could be expanded to better describe the purpose of the tests.

[Kareiman]

The purpose of the edge, classical cases and defensive tests is explained below

[Frank2446-dotcom]

Improvement: The test class name TestConvertCapitalLetters is clear, but it could be improved to use a more conventional PascalCase naming style.

Action: Use PascalCase for the test class name, e.g., TestConvertToCapital.

class TestConvertToCapital(unittest.TestCase):

[Kareiman]

done

[Frank2446-dotcom]

Well done

[Emanfalouji]

Great work kariman ..