Skip to content

Conversation

@gai93003
Copy link

@gai93003 gai93003 commented Oct 20, 2025

Learners, PR Template

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

@github-actions
Copy link

Your PR description contained template fields which weren't filled in.

Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

@gai93003 gai93003 added 🏕 Priority Mandatory This work is expected Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Decomposition The name of the module. labels Oct 20, 2025
@github-actions
Copy link

Your PR description contained template fields which weren't filled in.

Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

7 similar comments
@github-actions
Copy link

Your PR description contained template fields which weren't filled in.

Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

@github-actions
Copy link

Your PR description contained template fields which weren't filled in.

Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

@github-actions
Copy link

Your PR description contained template fields which weren't filled in.

Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

@github-actions
Copy link

Your PR description contained template fields which weren't filled in.

Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

@github-actions
Copy link

Your PR description contained template fields which weren't filled in.

Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

@github-actions
Copy link

Your PR description contained template fields which weren't filled in.

Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

@github-actions
Copy link

Your PR description contained template fields which weren't filled in.

Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

@LonMcGregor LonMcGregor added the Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. label Oct 29, 2025
Copy link

@LonMcGregor LonMcGregor left a comment

Choose a reason for hiding this comment

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

Remember to leave a summary of what work you've done in the PR body on github.

You don't need to commit the__pycache__ directory

Good start on this sprint's tasks, I have spotted a few areas where you could improve code further

@@ -0,0 +1,5 @@
def double(n):
return n * 3

Choose a reason for hiding this comment

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

Does this function actually double? What change could you make to resolve the issue?

Copy link
Author

Choose a reason for hiding this comment

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

I think for this function, there are two possible interpretations. Firstly, the function might have been meant to return the triple value of n. In this case, I could the function name from double(n) to triple(n). Secondly, the function might actually been meant to return double the value of n. Meaning I can just change the 3 to 2.

@@ -0,0 +1,30 @@
def open_account(balances, name, amount):

Choose a reason for hiding this comment

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

Part of the bank account task involved making type annotations, can you try that?

Copy link
Author

Choose a reason for hiding this comment

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

I have just added the type annotations on the functions

Choose a reason for hiding this comment

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

did you try running the code and spotting what the error was?

self.preferred_operating_system = preferred_operating_system
self.address = address

def is_adult(self):

Choose a reason for hiding this comment

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

What is a benefit or drawback of writing the function this way?

Copy link
Author

Choose a reason for hiding this comment

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

Writing a method has many benefits, such as it helps in encapsulating logic, as a result, it makes the method reusable in other instances of the class. The problem of these functions is that they become difficult to test in isolation as compared to free functions.

def is_adult(self):
today_date = date.today()
birth_date = date.fromisoformat(self.date_of_birth)
age = today_date.year - birth_date.year

Choose a reason for hiding this comment

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

Would this date calculation work all the time? What about if they are one day away from a 18th birthday in the current year?

Copy link
Author

Choose a reason for hiding this comment

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

This calculation will not work in this scenario because I am only comparing the years. I have changed the functionality to include the days instead of only comparing the years

class Person:
name: str
children: List["Person"]
age: int

Choose a reason for hiding this comment

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

Can you solve this in a way that it always prints the current age, rather than hard-coding a specific age?

Copy link
Author

Choose a reason for hiding this comment

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

I have changed the age to date_of_birth and used the function i used previous to calculate the age of the person as long as the birthday is given.

@LonMcGregor LonMcGregor added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Oct 29, 2025
@LonMcGregor
Copy link

Good work, I think this is complete now

@LonMcGregor LonMcGregor added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Nov 3, 2025
@github-actions
Copy link

github-actions bot commented Nov 3, 2025

Your PR couldn't be matched to an assignment in this module.

Please check its title is in the correct format, and that you only have one PR per assignment.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

1 similar comment
@github-actions
Copy link

github-actions bot commented Nov 3, 2025

Your PR couldn't be matched to an assignment in this module.

Please check its title is in the correct format, and that you only have one PR per assignment.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. Module-Decomposition The name of the module. 🏕 Priority Mandatory This work is expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants