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

Challenge/1 even odd range rouaa #52

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
c80c985
Add files via upload
Rouaa93 Jan 6, 2025
187b635
Add files via upload
Rouaa93 Jan 6, 2025
9f85f1a
Add files via upload
Rouaa93 Jan 6, 2025
17ab504
Add files via upload
Rouaa93 Jan 6, 2025
a103249
file update
Rouaa93 Jan 8, 2025
e3458c6
Resolve conflict in even_odd.py
Rouaa93 Jan 10, 2025
3663ff5
Merge branch 'challenge/1-even_odd_range-Rouaa' of https://github.com…
Rouaa93 Jan 10, 2025
d7924fb
update chenges
Rouaa93 Jan 11, 2025
3cfec3a
Delete solutions/Group 17.code-workspace
Aseel-AbuKmail Jan 11, 2025
63eebd3
Delete solutions/tests/test_rectangle_area.py
Aseel-AbuKmail Jan 11, 2025
0a2a47c
Delete solutions/test_even_odd_range.py
Aseel-AbuKmail Jan 11, 2025
b555315
Delete solutions/rectangle_area.py
Aseel-AbuKmail Jan 11, 2025
2870933
Update and rename test_even_odd_range.py to test_even_odd.py
Aseel-AbuKmail Jan 11, 2025
c787167
Update def even_odd_range(start: int, end: int) -> Tuple[List[int], L…
Aseel-AbuKmail Jan 11, 2025
861d1ea
Update def test_even_odd_range_negative_numbers(self):
Aseel-AbuKmail Jan 11, 2025
c1be394
Rename even_odd.py to even_odd_range.py
Aseel-AbuKmail Jan 12, 2025
43b797a
Update and rename test_even_odd.py to test_even_odd_range.py
Aseel-AbuKmail Jan 12, 2025
1dd14d1
Update from ..even_odd import even_odd_range to from ..even_odd_range…
Aseel-AbuKmail Jan 12, 2025
b523768
Update test_even_odd_range.py
Rouaa93 Jan 12, 2025
2953217
Update test_even_odd_range.py
Rouaa93 Jan 12, 2025
9ba9be7
Update test_even_odd_range.py
Rouaa93 Jan 12, 2025
8fa4c1e
Update test_even_odd_range.py
Rouaa93 Jan 12, 2025
a13cab3
Update test_even_odd_range.py
Rouaa93 Jan 12, 2025
6e1833e
Update even_odd_range.py
Rouaa93 Jan 12, 2025
3e326fd
Update test_even_odd_range.py
Rouaa93 Jan 12, 2025
1f90043
Update test_even_odd_range.py
Rouaa93 Jan 13, 2025
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
53 changes: 53 additions & 0 deletions solutions/even_odd.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
"""
This module contains a function that takes a range of numbers and returns
two lists: one for even numbers and one for odd numbers within the specified range.

Function:
even_odd_range(start: int, end: int) -> tuple[list[int], list[int]]

Arguments:
start (int): The starting number of the range (inclusive).
end (int): The ending number of the range (inclusive).

Returns:
tuple[list[int], list[int]]: A tuple containing two lists:
- A list of even numbers in the range.
- A list of odd numbers in the range.

Raises:
ValueError: If start or end are not integers.
ValueError: If start is greater than end.
"""

from typing import List, Tuple


def even_odd_range(start: int, end: int) -> Tuple[List[int], List[int]]:
"""
Returns two lists: one with the even numbers and one with the odd numbers
within the specified range.

Arguments:
start -- The starting number (inclusive)
end -- The ending number (inclusive)

Returns:
A tuple of two lists: the first list contains even numbers, the second contains odd numbers.
"""

Choose a reason for hiding this comment

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

The function docstring is well written and shows clearly how the function is used but it can be improved even more. Consider adding the strategy to help others better understand how the function works before even going into the details of the code.
Also, make sure to add doctests to the function docstring to provide examples on the output of the function


# Defensive assertions
if not isinstance(start, int) or not isinstance(end, int):
raise ValueError("Both start and end must be integers.")
if start > end:
raise ValueError("The start value must not be greater than the end value.")

even_numbers = []
odd_numbers = []

for num in range(start, end + 1):
if num % 2 == 0:
even_numbers.append(num)
else:
odd_numbers.append(num)

return even_numbers, odd_numbers
44 changes: 44 additions & 0 deletions solutions/tests/test_even_odd_range.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import unittest

Choose a reason for hiding this comment

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

Your test file is also well written, and I liked how each function has a descriptive docstring and that me it easier for my to understand the purpose of each test. Overall, the quality is high and it can be even better with some changes.

  • Add a module docstring to describe the purpose of the tests, what is being tested and what types of test cases are included in the file. This will make the test file even more easy to understand.
  • Consider adding more tests for large ranges
  • Consider adding a test for negative numbers to ensure your cover most of the cases
  • I also noticed that two tests serve the same purpose:
def test_even_odd_range_invalid_order(self):
        """
        Tests that ValueError is raised if start is greater than end.
        """
        with self.assertRaises(ValueError):
            even_odd_range(5, 3)
def test_even_odd_range_empty(self):
        """
        Tests that the function raises an error for an empty range where start > end.
        """
        with self.assertRaises(ValueError):
            even_odd_range(5, 4)

They both check when the start is greater than the end and raise a ValueError and this seems redundant, and you can consider dropping one of the tests. I suggest keeping the first function since it describes the scenario better.

Copy link
Author

Choose a reason for hiding this comment

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

@abdoalsir

Thank you so much, Abdo, for your detailed feedback and constructive suggestions!
I really appreciate you pointing out the strengths in the code and offering ways to improve it further. Here’s how I plan to address your comments:
1. Function Docstring and Doctests: I’ll update the function docstring to include more details about the strategy, making it easier for others to understand how it works at a glance. I’ll also add doctests to show examples of the function’s behavior.
2. Test File Improvements:
• I’ll add a module docstring to explain the purpose of the test file and summarize the types of cases it covers.
• I’ll include tests for larger ranges and negative numbers to make the test cases more comprehensive.
• Regarding the redundant tests, I’ll remove the test_even_odd_range_empty test as you suggested and keep the test_even_odd_range_invalid_order since it’s clearer.

Once I make these changes, I hope the code will be more polished and easier to understand. Thanks again for taking the time to review and for your helpful insights—it really means a lot! 😃👌.


from ..even_odd import even_odd_range


class TestEvenOddRange(unittest.TestCase):
"""
Test case for the even_odd_range function.
"""

def test_even_odd_range_valid(self):
"""
Tests that the function correctly splits a range into even and odd numbers.
"""
result = even_odd_range(1, 5)
self.assertEqual(result, ([2, 4], [1, 3, 5]))

def test_even_odd_range_empty(self):
"""
Tests that the function raises an error for an empty range where start > end.
"""
with self.assertRaises(ValueError):
even_odd_range(5, 4)

def test_even_odd_range_single(self):
"""
Tests that a single number in the range is classified correctly.
"""
result = even_odd_range(4, 4)
self.assertEqual(result, ([4], []))

def test_even_odd_range_invalid_type(self):
"""
Tests that ValueError is raised if start or end are not integers.
"""
with self.assertRaises(ValueError):
even_odd_range(1, "5")

def test_even_odd_range_invalid_order(self):
"""
Tests that ValueError is raised if start is greater than end.
"""
with self.assertRaises(ValueError):
even_odd_range(5, 3)
Loading