-
-
Notifications
You must be signed in to change notification settings - Fork 61
Glasgow | Sheetal Kharab | Module-Decomposition | Sprint 4 | prep-exercises #28
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
Changes from 10 commits
35cb2cb
239f500
cb6fadc
2d7f0a4
5b25f8b
2235109
efe7029
d2fb86d
c7955bf
1cd6775
974852d
c7bb701
b337a40
3cf2111
7ca2702
d8cdada
806097c
0a3bae2
49b3dba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| node_modules/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| def open_account(balances, name, amount): | ||
| balances[name] = amount | ||
|
|
||
| def sum_balances(accounts): | ||
| total = 0 | ||
| for name, pence in accounts.items(): | ||
| print(f"{name} had balance {pence}") | ||
| total += pence | ||
| return total | ||
|
|
||
| def format_pence_as_string(total_pence): | ||
| if total_pence < 100: | ||
| return f"{total_pence}p" | ||
| pounds = int(total_pence / 100) | ||
| pence = total_pence % 100 | ||
| return f"£{pounds}.{pence:02d}" | ||
|
|
||
| balances = { | ||
| "Sima": 700, | ||
| "Linn": 545, | ||
| "Georg": 831, | ||
| } | ||
| #missed balance argument, add balances | ||
| open_account(balances,"Tobi", 913) | ||
| #3rd argument is an int not a String and format as pence | ||
| open_account(balances,"Olya", 713) | ||
|
|
||
| total_pence = sum_balances(balances) | ||
| #wrong name of function | ||
| total_string = format_pence_as_string(total_pence) | ||
|
|
||
| print(f"The bank accounts total {total_string}") | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| from dataclasses import dataclass | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In python, the naming convention for files is
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sorry, I changed them thank you |
||
| from datetime import date | ||
|
|
||
| @dataclass(frozen=True) | ||
| class Person: | ||
| name: str | ||
| date_of_birth: date | ||
| preferred_operating_system: str | ||
|
|
||
| def is_adult(self): | ||
| today_date = date.today() | ||
| birth_date= self.date_of_birth | ||
| #to check date and month as well | ||
| age = today_date.year-birth_date.year- ((today_date.month, today_date.day) < (birth_date.month, birth_date.day)) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the readability could be improved if you use date arithmetics here, for example (assuming and compare it with the date of birth
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i tried to follow your suggestion and made changes. |
||
| return age>=18 | ||
|
|
||
| imran = Person("Imran", (1989,6,25), "Ubuntu") # We can call this constructor - @dataclass generated it for us. | ||
| print(imran) # Prints Person(name='Imran', date_of_birth=(1989, 6, 25), preferred_operating_system='Ubuntu') | ||
|
|
||
| imran2 = Person("Imran", (1989,6,25), "Ubuntu") | ||
| print(imran == imran2) # Prints True | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| def half(value): | ||
| return value / 2 | ||
|
|
||
| def double(value): | ||
| return value * 2 | ||
|
|
||
| def second(value): | ||
| return value[1] | ||
|
|
||
|
|
||
|
|
||
| print(double(22)) | ||
| print(double("hello")) | ||
| print(double("22")) | ||
|
|
||
| # In Python when we multiply a string by a number, it repeats the string that many times(in our case it is 2 times). | ||
| # i got in answer 2222 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| from dataclasses import dataclass | ||
| from enum import Enum | ||
| import sys | ||
| from typing import List, Optional | ||
|
|
||
| class OperatingSystem(Enum): | ||
| MACOS = "macOS" | ||
| ARCH = "Arch Linux" | ||
| UBUNTU = "Ubuntu" | ||
|
|
||
| @dataclass(frozen=True) | ||
| class Person: | ||
| name: str | ||
| age: int | ||
| preferred_operating_system: OperatingSystem | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class Laptop: | ||
| id: int | ||
| manufacturer: str | ||
| model: str | ||
| screen_size_in_inches: float | ||
| operating_system: OperatingSystem | ||
|
|
||
|
|
||
| def find_possible_laptops(laptops: List[Laptop], person: Person) -> List[Laptop]: | ||
| possible_laptops = [] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just for you information, an alternative approach could also be
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the suggestion! yes a list comprehension is more concise and readable |
||
| for laptop in laptops: | ||
| if laptop.operating_system == person.preferred_operating_system: | ||
| possible_laptops.append(laptop) | ||
| return possible_laptops | ||
|
|
||
| def parse_operating_system(s: str) -> Optional[OperatingSystem]: | ||
| s_normal = s.strip().lower() | ||
| for os in OperatingSystem: | ||
| if os.value.lower() == s_normal: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a comment (no need to fix) - you to made it case insensitive so it works (all good), but overall you could also use https://docs.python.org/3/library/enum.html#enum.EnumType.__getitem__
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the note!I’ll keep that in mind for future refactoring. |
||
| return os | ||
| return None | ||
|
|
||
|
|
||
| people = [ | ||
| Person(name="Imran", age=22, preferred_operating_system=OperatingSystem.UBUNTU), | ||
| Person(name="Eliza", age=34, preferred_operating_system=OperatingSystem.ARCH), | ||
| ] | ||
|
|
||
| laptops = [ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optional: you can consider using ruff to improve your code formatiing, something like:
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will do it definitely. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you get a chance to check this out? |
||
| Laptop(id=1, manufacturer="Dell", model="XPS", screen_size_in_inches=13, operating_system=OperatingSystem.ARCH), | ||
| Laptop(id=2, manufacturer="Dell", model="XPS", screen_size_in_inches=15, operating_system=OperatingSystem.UBUNTU), | ||
| Laptop(id=3, manufacturer="Dell", model="XPS", screen_size_in_inches=15, operating_system=OperatingSystem.UBUNTU), | ||
| Laptop(id=4, manufacturer="Apple", model="macBook", screen_size_in_inches=13, operating_system=OperatingSystem.MACOS), | ||
| ] | ||
|
|
||
|
|
||
|
|
||
| while True: | ||
| name = input("Enter your name: ") | ||
| if name == "": | ||
| print("Error: Name cannot be empty. Please try again.") | ||
| else: | ||
| break | ||
|
|
||
| while True: | ||
| age_input = input("Enter your age: ") | ||
| if not age_input.isdigit(): | ||
| print("Error: Age must be a number.", file=sys.stderr) | ||
| continue | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would consider throwing a value error here and using try/catch block instead.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you, try to follow your suggestion and made changes |
||
| age = int(age_input) | ||
| if age < 0: | ||
| print("Error: Age cannot be negative.", file=sys.stderr) | ||
| continue | ||
| break | ||
|
|
||
| while True: | ||
| os_input = input("Preferred operating system (macOS, Arch Linux, Ubuntu): ").strip() | ||
| preferred_os = parse_operating_system(os_input) | ||
| if preferred_os is not None: | ||
| break | ||
| print("Please choose one of: macOS, Arch Linux, Ubuntu.") | ||
|
|
||
|
|
||
|
|
||
|
|
||
| # Create the Person with validated types | ||
| person = Person(name=name, age=age, preferred_operating_system=preferred_os) | ||
| people.append(person) | ||
|
|
||
| # Find matching laptops and print count | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of adding comments, try to see where it make sense to group these various bits of logic into methods with descriptive names.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I try to make changes and use different function There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! In general, apart from invoking the main method, all the logic needs to be part of some method / class. The clarity should be coming from the name of the methods and structure of the code rather than comments explaining what you are doing (those could be useful too in some cases but should not be used as a substitute for clear structure and naming). That said, from what I see this is more like a hands-on exercise to practice some concepts so it is ok your code has less structure. |
||
| for person in people: | ||
| matching_laptops = find_possible_laptops(laptops, person) | ||
| print(f"\nHi {person.name}, we found {len(matching_laptops)} laptop(s) acc. your preferred OS ({person.preferred_operating_system.value}).") | ||
|
|
||
| # Count how many laptops exist per OS | ||
| mac_count = 0 | ||
| arch_count = 0 | ||
| ubuntu_count = 0 | ||
|
|
||
| for laptop in laptops: | ||
| if laptop.operating_system == OperatingSystem.MACOS: | ||
| mac_count += 1 | ||
| elif laptop.operating_system == OperatingSystem.ARCH: | ||
| arch_count += 1 | ||
| elif laptop.operating_system == OperatingSystem.UBUNTU: | ||
| ubuntu_count += 1 | ||
|
|
||
| # Find OS with the most laptops | ||
| best_os = OperatingSystem.MACOS | ||
| best_count = mac_count | ||
|
|
||
| if arch_count > best_count: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should design it in a way that doesn't require you to go through all possible values manually. Imagine your OperatingSystem enum containing 50 different systems. Try to use, for example, a dict structure.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i tried to follow your suggestion |
||
| best_os = OperatingSystem.ARCH | ||
| best_count = arch_count | ||
|
|
||
| if ubuntu_count > best_count: | ||
| best_os = OperatingSystem.UBUNTU | ||
| best_count = ubuntu_count | ||
|
|
||
| # Compare availability and suggest better OS | ||
| preferred_count = 0 | ||
| if person.preferred_operating_system == OperatingSystem.MACOS: | ||
| preferred_count = mac_count | ||
| elif person.preferred_operating_system == OperatingSystem.ARCH: | ||
| preferred_count = arch_count | ||
| elif person.preferred_operating_system == OperatingSystem.UBUNTU: | ||
| preferred_count = ubuntu_count | ||
|
|
||
| if best_os != person.preferred_operating_system and best_count > preferred_count: | ||
| print(f"\n More laptops are available if you choose {best_os.value} ({best_count} available).") | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| from dataclasses import dataclass | ||
| from typing import List | ||
|
|
||
| @dataclass(frozen=True) | ||
| class Person: | ||
| name: str | ||
| age: int | ||
| children: List["Person"] | ||
|
|
||
| fatma = Person(name="Fatma",age=11, children=[]) | ||
| aisha = Person(name="Aisha", age=13,children=[]) | ||
|
|
||
| imran = Person(name="Imran", age=40, children=[fatma, aisha]) | ||
|
|
||
| def print_family_tree(person: Person) -> None: | ||
| print(person.name) | ||
| for child in person.children: | ||
| print(f"- {child.name} ({child.age})") | ||
|
|
||
| print_family_tree(imran) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
|
|
||
| from typing import List | ||
|
|
||
|
|
||
| class Parent: | ||
| def __init__(self, first_name: str, last_name: str): | ||
| self.first_name = first_name | ||
| self.last_name = last_name | ||
|
|
||
| def get_name(self) -> str: | ||
| return f"{self.first_name} {self.last_name}" | ||
|
|
||
|
|
||
| class Child(Parent): | ||
| def __init__(self, first_name: str, last_name: str): | ||
| super().__init__(first_name, last_name) | ||
| # for type annotation as its a list of string | ||
| self.previous_last_names : List[str]= [] | ||
|
|
||
| def change_last_name(self, last_name) -> None: | ||
| self.previous_last_names.append(self.last_name) | ||
| self.last_name = last_name | ||
|
|
||
| def get_full_name(self) -> str: | ||
| suffix = "" | ||
| if len(self.previous_last_names) > 0: | ||
| suffix = f" (née {self.previous_last_names[0]})" | ||
| return f"{self.first_name} {self.last_name}{suffix}" | ||
|
|
||
| person1 = Child("Elizaveta", "Alekseeva") | ||
| print(person1.get_name()) | ||
| print(person1.get_full_name()) | ||
| person1.change_last_name("Tyurina") | ||
| print(person1.get_name()) | ||
| print(person1.get_full_name()) | ||
|
|
||
| person2 = Parent("Elizaveta", "Alekseeva") | ||
| print(person2.get_name()) | ||
| # Need to Commented out because the Parent class doesn't have 'get_full_name()' and 'change_last_name()' methods | ||
| #print(person2.get_full_name()) | ||
| #person2.change_last_name("Tyurina") | ||
| print(person2.get_name()) | ||
| #print(person2.get_full_name()) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| from dataclasses import dataclass | ||
| from typing import List | ||
|
|
||
| @dataclass(frozen=True) | ||
| class Person: | ||
| name: str | ||
| age: int | ||
| preferred_operating_systems: List[str] | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class Laptop: | ||
| id: int | ||
| manufacturer: str | ||
| model: str | ||
| screen_size_in_inches: float | ||
| operating_system: str | ||
|
|
||
|
|
||
| def find_possible_laptops(laptops: List[Laptop], person: Person) -> List[Laptop]: | ||
| possible_laptops = [] | ||
| for laptop in laptops: | ||
| if laptop.operating_system == person.preferred_operating_systems: | ||
| possible_laptops.append(laptop) | ||
| return possible_laptops | ||
|
|
||
|
|
||
| people = [ | ||
| Person(name="Imran", age=22, preferred_operating_systems=["Ubuntu"]), | ||
| Person(name="Eliza", age=34, preferred_operating_systems=["Arch Linux"]), | ||
| ] | ||
|
|
||
| laptops = [ | ||
| Laptop(id=1, manufacturer="Dell", model="XPS", screen_size_in_inches=13, operating_system="Arch Linux"), | ||
| Laptop(id=2, manufacturer="Dell", model="XPS", screen_size_in_inches=15, operating_system="Ubuntu"), | ||
| Laptop(id=3, manufacturer="Dell", model="XPS", screen_size_in_inches=15, operating_system="ubuntu"), | ||
| Laptop(id=4, manufacturer="Apple", model="macBook", screen_size_in_inches=13, operating_system="macOS"), | ||
| ] | ||
|
|
||
| for person in people: | ||
| possible_laptops = find_possible_laptops(laptops, person) | ||
| print(f"Possible laptops for {person.name}: {possible_laptops}") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| from datetime import date | ||
|
|
||
|
|
||
| class Person: | ||
| # a constructor with a new instance of class | ||
| def __init__(self, name: str, date_of_birth:date, preferred_operating_system: str): | ||
| self.name = name | ||
| self.date_of_birth = date_of_birth | ||
| self.preferred_operating_system = preferred_operating_system | ||
|
|
||
| def is_adult(self): | ||
| today_date = date.today() | ||
| birth_date= self.date_of_birth | ||
| #to check date and month as well | ||
| age = today_date.year-birth_date.year- ((today_date.month, today_date.day) < (birth_date.month, birth_date.day)) | ||
| return age>=18 | ||
|
|
||
|
|
||
| imran = Person("Imran", date(1989,6,25), "Ubuntu") | ||
| print(imran.name) | ||
| print(imran.is_adult()) | ||
|
|
||
| eliza = Person("Eliza", date(2007,10,12), "Arch Linux") | ||
| print(eliza.is_adult()) | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point of the exercise is not to just find a bug, but use mypy to do that. Could you please add types to this file and others as well? You will need to add mypy to requirements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review. I have already added type annotations to all functions in this file and other related files and used mypy to check for type consistency. The mypy check shows Success: no issues found.
I realized I had forgotten to include mypy in requirements.txt, but I’ve now added it so it will be installed automatically.