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

Swap_letters review #48

Merged
merged 2 commits into from
Jan 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions solutions/Swap_letters.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
"""
Created on 08/01/2024

@author: Tibyan Khalid

This file contains the function Swap_letters that swaps the case of each letter in a string.
"""

Choose a reason for hiding this comment

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

The docstring is good for keeping track of the file's creation and author. However, it would be more helpful if you included a brief description of the file's purpose or functionality here. For example:

"""This file contains the function Swap_letters that swaps the case of each letter in a string."""

Copy link
Author

Choose a reason for hiding this comment

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

Good idea yeah okay I'll do that now



def Swap_letters(string: str) -> str:
"""Swap_letters will return the string given with switched cases.

Parameters:
String(str) "the word that will get modified"

Returns: modified_string(str) "the string with swapped cases"

>>> Swap_letters("hello")
'HELLO'

>>> Swap_letters("HELLO")
'hello'

>>> Swap_letters("HeLlO")
'hElLo'
"""

Choose a reason for hiding this comment

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

The docstring is clear and provides example usage (which is good for doctests). However:

Add edge cases in the doctests, such as empty strings, strings with digits, or special characters.

Copy link
Author

Choose a reason for hiding this comment

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

I've added the edge cases tests in the tests file already

if not isinstance(string, str):
"Make sure the expected input is a string"

Choose a reason for hiding this comment

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

The string literal "Make sure the expected input is a string" doesn’t do anything in the code. It should either be removed or converted to a comment:

Make sure the expected input is a string

Copy link
Author

Choose a reason for hiding this comment

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

It's just for documenting purposes, it acts as a comment

raise AssertionError("Input must be a string")

changed_string = ""
for char in string:
if char.islower():
changed_string += char.upper()
elif char.isupper():
changed_string += char.lower()
else:
changed_string += char
return changed_string

Choose a reason for hiding this comment

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

The function implementation is clear, but you can simplify the concatenation in the loop:
changed_string += char.upper() if char.islower() else char.lower() if char.isupper() else char

Using += is cleaner and avoids multiple concatenations.

Copy link
Author

Choose a reason for hiding this comment

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

Am I the only one who doesn't like " += " haha, but yeah I think it definitely is clearer I'll change it now

49 changes: 49 additions & 0 deletions solutions/tests/test_Swap_letters.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
"""
Created on 08/01/2024

@author: Tibyan Khalid

This file contains boundary cases and defensive assertion tests for the function Swap_letters.
"""

Choose a reason for hiding this comment

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

Same as the previous file—consider adding a description of what the test file is testing

Copy link
Author

Choose a reason for hiding this comment

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

Okay sure


import unittest

from ..Swap_letters import Swap_letters


class TestSwapLettersFunctionality(unittest.TestCase):
"""Test cases for validating the behavior of the Swap_letters function,
including edge cases and defensive assertions."""

def test_lowercase_all(self):
"Testing from lowercase to uppercase"
self.assertEqual(Swap_letters("tibyan"), "TIBYAN")

Choose a reason for hiding this comment

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

It’s a good idea to add edge case tests, such as testing an empty string or special characters.
You could consider adding more boundary tests for edge cases (empty strings, non-letters).

Copy link
Author

Choose a reason for hiding this comment

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

I thought I added them omg, okay I'll add them right nowww


def test_uppercase_all(self):
"Testing from uppercase to lowercase"
self.assertEqual(Swap_letters("TIBYAN"), "tibyan")

def test_mixed_cases(self):
"Testing mixed spaces"
self.assertEqual(Swap_letters("TiByAn"), "tIbYaN")

def test_non_string_entry(self):
"Raise an error if entry is not a string"
with self.assertRaises(AssertionError):
Swap_letters(57)

def test_spaces(self):
"Handle spaces correctly"
self.assertEqual(Swap_letters("Hello World"), "hELLO wORLD")

def test_empty_string(self):
"Test for an empty string input"
self.assertEqual(Swap_letters(""), "")

def test_special_characters(self):
"Test for special characters input"
self.assertEqual(Swap_letters("1234!@#$"), "1234!@#$")


if __name__ == "__main__":
unittest.main()
Loading