Todo application implementation

Posted on

Problem

I am new to c# and decided to write a classic todo app. The implementation of my application now is like this: there is a Task class that describes the task. The List class acts simultaneously as a storage and has methods for creating, deleting, and displaying tasks. How good is this approach, how can you do better (if possible)? I would like a full code review.

There is Program.cs:

using System;

namespace ToDo
{
    class Program
    {
        static void Main(string[] args)
        {
            List list = new List();
            list.AddTask("Task №1");
            list.AddTask("Task №2");
            list.AddTask("Task №3");
            list.AddTask("Task №4");
            list.AddTask("Task №5");
            list.WriteTasks();
            list.RemoveTask(2);
            list.WriteTasks();
        }
    }
}

List.cs:

using System;
using System.Collections.Generic;
using System.Text;

namespace ToDo
{
    class List
    {
        private List<Task> tasks = new List<Task>();
        public void AddTask(string body)
        {
            tasks.Add(new Task() { Body = body });
        }
        public void WriteTasks()
        {
            for (int i = 0; i < tasks.Count; i++)
            {
                Console.WriteLine($"[{i+1}]: {tasks[i].Body}");
            }

            Console.ReadKey();
        }
        public void RemoveTask(int number)
        {
            for (int i = 0; i < tasks.Count; i++)
            {
                if (number == i + 1)
                {
                    tasks.RemoveAt(i);
                }
            }
        }
    }
}

Task.cs:

using System;
using System.Collections.Generic;
using System.Text;

namespace ToDo
{
    class Task
    {
        public string Body { get; set; }
    }
}

Solution

As a very basic application this is fine, however, if the program becomes more complicated and you want to add a data base and asynchronous programming you will run into conflicts with some of the class names. For instance the Task class is supplied by the C# library for asynchronous programming.

If I was going to implement a task list application I would have a few more properties for the task class, such as:

  • Priority
  • Status (not started, started, in progress, completed).
  • Due date
  • Name
  • Description

These are the minimum I would use, some additional properties might be

  • Assigned to
  • Assigned by

if this was a multi-user task list.

I agree with what pacmaninbw says, just a couple more things:

For a task object the minimum I’d have is an Id property (unique, make it a private set and initialize it in the constructor), Name (doesn’t have to be unique but might be better from a user perspective), and Description (doesn’t need to be unique). If you end up storing this in a database you’ll probably index on the Id property.

If you end up adding more properties to the task I’d recommend overriding the ToString() method to print out a more detailed description of the object in your WriteTask method.

I see there’s an emphasis on the index of the task (WriteTasks prints out the index of the task, RemoveTask requires an index to remove the task). A user is not going to care about where in the list a task is stored – for example they will most likely want to delete by name, id, etc.

WriteTasks can be simplified to

tasks.ForEach(x => Console.WriteLine(x.Body));

and RemoveTask can be simplified to something like the below, no need for the iteration.

if (number <= list.Count()) {
    list.RemoveAt(number - 1);
}

Leave a Reply

Your email address will not be published. Required fields are marked *