Adding uppercase 2

[Explotion80]

Adding uppercase 2

[github-actions]

Here is a focused review of the diff you provided, with problems found, suggestions for improvement, and a corrected/refactored example.

Summary

• A new function to_uppercase2 was added but contains multiple bugs: it returns the wrong transformation (lowercase), has unreachable code after return, and contains a misspelled variable name (lenght).
• CLI behavior is very minimal; consider using argparse and better handling of multiple words and error output.
• Module could use basic docstrings, simple validation, and unit tests.
• There was previously no newline at end of file; the diff now adds one (good).

Detailed issues and recommendations

1. Function behavior and naming

• Problem: to_uppercase2 is named as if it would return uppercase but it returns text.lower(). This is misleading.
• Recommendation: either rename to to_lowercase or change implementation to return text.upper() if the intention is another uppercase variant.

2. Unreachable code and misspelling

• Problem: the line lenght = len(text) appears after return text.lower() — it's unreachable and the variable name is misspelled.
• Recommendation: Remove unreachable code. If you need the length, compute it before returning and use a correct name (length).

3. Dead code / leftover code smell

• Problem: to_uppercase2 appears to be an accidental or unfinished helper. Unused functions should be removed or completed and covered by tests.
• Recommendation: Remove it if unused, or implement it properly (with a docstring) and add tests.

4. CLI improvements

• Problem: The script only uses sys.argv[1] and prints usage to stdout. No support for multiple words, no helpful exit codes, no option parsing.
• Recommendation:
  • Use argparse for clearer CLI (support passing multiple words, flags).
  • Print usage and error messages to stderr and exit with non-zero status on error.
  • Consider exposing both uppercase and lowercase options (or a --case flag).

5. Type handling and robustness

• Problem: calling .upper() on non-string will raise AttributeError. Type hints are helpful but not enforced at runtime.
• Recommendation: either validate isinstance(text, str) and raise TypeError with a clear message, or coerce via str(text) depending on intended semantics.

6. Style and documentation

• Recommendation: Add a module docstring and function docstrings. Keep code PEP8 compliant (the snippet is small so mostly OK).
• Add unit tests (pytest) for to_uppercase and any other public functions.

7. Testing / CI

• Suggest adding a small test file (tests/test_uppercase.py) that covers typical and edge cases (empty string, non-string input if supported).

Suggested corrected/refactored implementation

• This example:
  • Keeps the original to_uppercase
  • Adds a correctly-named to_lowercase
  • Adds a main() using argparse that supports multiple words and selects case via an optional flag
  • Validates input by coercing to str

Example code (drop-in replacement):

"""
Simple utilities to change case of text and a small CLI.
"""

from __future__ import annotations
import argparse
import sys


def to_uppercase(text: str) -> str:
    """Return text converted to uppercase."""
    return text.upper()


def to_lowercase(text: str) -> str:
    """Return text converted to lowercase."""
    return text.lower()


def main(argv: list[str] | None = None) -> int:
    """CLI entry point. Returns exit code."""
    parser = argparse.ArgumentParser(description="Convert text to upper- or lowercase.")
    parser.add_argument("text", nargs="+", help="Text to convert (one or more words)")
    parser.add_argument("--lower", action="store_true", help="Convert to lowercase instead of uppercase")
    args = parser.parse_args(argv)

    text = " ".join(args.text)
    # if you want to enforce strings: text = str(text)

    result = to_lowercase(text) if args.lower else to_uppercase(text)
    print(result)
    return 0


if __name__ == "__main__":
    raise SystemExit(main())

Notes about the example

• Uses argparse so usage/help are handled automatically.
• Joins argv words so multi-word input works without quoting.
• Explicit main() returns exit code, and SystemExit is raised for proper exit codes.
• Added to_lowercase instead of the broken to_uppercase2.

Suggested unit test (pytest)

def test_to_uppercase():
    assert to_uppercase("abc") == "ABC"
    assert to_uppercase("") == ""
    assert to_uppercase("123!") == "123!"

def test_to_lowercase():
    assert to_lowercase("AbC") == "abc"

Final checklist before committing

• Remove or fix to_uppercase2 (the current diff adds a broken function).
• Add docstrings and tests.
• Consider switching CLI to argparse and making usage messages go to stderr when appropriate.
• Ensure file ends with a newline (now present).
• Run linters (flake8/ruff) and type checker (mypy) if used in your project.

If you want, I can:

• Provide a patch that replaces the file with the refactored code above.
• Create pytest-compatible tests.
• Provide minimal argparse-less quick fix instead (rename to_lowercase and remove unreachable line).