Skip to content

Conversation

@jhill1
Copy link
Owner

@jhill1 jhill1 commented Apr 28, 2025

Nice work!

Things you did well:

  • nice, clean readable code
  • passes most tests
  • good comments and coding style

Where you could improve:

  • commit history is mixed. There are some good, clean commits, but others change to much at once and overwrite things that should be
  • Your code failed two tests with insufficient output from --list option
  • add copyright and licence
  • Feel free to embellish and add functionality

import os

TASK_FILE = ".tasks.txt"
TASK_FILE = "todo_list.txt"
Copy link
Owner Author

Choose a reason for hiding this comment

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

dont edit this...

python/todo.py Outdated
counter = 1
output_string = ""

for i, task in enumerate(tasks):
Copy link
Owner Author

Choose a reason for hiding this comment

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

this could be done more efficiently, but is clear, which is the main thing.

python/todo.py Outdated
output_string = output_string + str(counter) + ". " + task
counter = counter + 1
return output_string
file.close()
Copy link
Owner Author

Choose a reason for hiding this comment

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

don't need file.close() with with statement.

python/todo.py Outdated


"""Appends a task to the end of the todo list file."""
task="Item 6"
Copy link
Owner Author

Choose a reason for hiding this comment

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

This will only ever add "Item 6"...

python/todo.py Outdated
"""Lists tasks within the task file"""
with open(TASK_FILE, "r") as file:
tasks = file.readlines()
tasks = file.readlines()
Copy link
Owner Author

Choose a reason for hiding this comment

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

You've also made changes to list tasks in a commit about add_task.

@@ -1,3 +1,8 @@
"""
Copy link
Owner Author

Choose a reason for hiding this comment

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

Make sure commit only add things in the message, not extra stuff

def remove_task(index_str):
"""Removes a task by index."""
try:
index = int(index_str) # converts the string to an integer
Copy link
Owner Author

Choose a reason for hiding this comment

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

Don't need this if you'd left the int conversion in main.

@jhill1
Copy link
Owner Author

jhill1 commented Apr 28, 2025

@ZorahRajput Comments above on the formative.

@jhill1 jhill1 closed this Apr 28, 2025
@jhill1 jhill1 reopened this May 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants