To do list mini project

Question

I tried in this project to apply what I learned from OOP, using classes and properties, please advise for any best practice or any improvement.

Conclusion: I have a text file "ToDoList.txt" which has the following syntax Task,status for example: Shopping,0 cooking,1 pick up kids from school,2 do something,0

And I have two classes:

• TaskData class which only has properties and it will return the status in text(Completed or not completed)
• FilterData class which is going to filter the data according to the status

Main Program class:

namespace ExtractingDataFromTextFile
{
 class Program
 {
 static void Main()
 {
 //Save Results of text data and declaring the text file to 
 //Read Data from.
 TaskData textData = new TaskData();
 string path = "ToDoList.txt";
 //Create a string to read all data in file
 string[] allData = File.ReadAllLines(path);
 //Assign each data to TextData object properties
 foreach (var task in allData)
 {
 PrintData(textData, task);
 }
 FilterData filterData = new FilterData();
 //Filter to show Completed Tasks only
 string[] completedTasks = filterData.ShowCompleted(allData);
 Console.WriteLine("\nFilter only completed tasks:");
 foreach (var task in completedTasks)
 {
 PrintData(textData, task); 
 }
 //Filter to show uncompleted Taks only
 string[] unCompletedTasks = filterData.ShowUnCompleted(allData);
 Console.WriteLine("\nFilter only Un-completed tasks:");
 foreach (var task in unCompletedTasks)
 {
 PrintData(textData, task);
 }
 Console.ReadLine();
 }
 //This method will print data to the console
 private static void PrintData(TaskData textData, string task)
 {
 string[] lineData = task.Split(',');
 textData.Name = lineData[0];
 textData.Status = lineData[1];
 Console.WriteLine($"Task: {textData.Name}, Status:[{textData.Status}]");
 }
 }
}

TaskData class:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
namespace ExtractingDataFromTextFile
{
 class TaskData
 {
 private string _status;
 public string Name { get; set; }
 public string Status
 {
 get {return _status; }
 //The setter will convert status from number to text
 //0 --> Not completed
 //1 --> Completed
 set
 {
 if (value == "0")
 {
 _status = "Not completed";
 }
 else if(value =="1")
 {
 _status = "Completed";
 }
 else
 {
 _status = "Unknown";
 }
 }
 }
 }
}

FilterData class

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
namespace ExtractingDataFromTextFile
{
 class FilterData
 {
 private List<string> _filterData = new List<string>();
 //This function will filter the list to show only completed tasks
 public string[] ShowCompleted(string[] allData)
 {
 _filterData = new List<string>(); //Recreate the list to delete old filter
 foreach (var line in allData)
 {
 string[] lineData = line.Split(',');
 if (lineData[1] == "1")
 {
 //Completed task
 _filterData.Add($"{lineData[0]},{lineData[1]}");
 }
 }
 return _filterData.ToArray();
 }
 //This function will filter the list to show only un-completed tasks
 public string[] ShowUnCompleted(string[] allData)
 {
 _filterData = new List<string>(); //Recreate the list to deleted old filter
 foreach (var line in allData)
 {
 string[] lineData = line.Split(',');
 if (lineData[1] == "0")
 {
 //Un-Completed task
 _filterData.Add($"{lineData[0]},{lineData[1]}");
 }
 }
 return _filterData.ToArray();
 }
 }
}

• Mast: Please see What to do when someone answers. I have rolled back Rev 5 → 3.
• AMJ: @Mast Thank you, I followed the link and answered my question.

Answer 1

In PrintData method you are assigning values to TaskData object but you don't seem to do anything with it besides just printing it out.

I would parse the file into a List at the beginning and then work with this collection in later code.

private static List<TaskData> ParseTaskData(string[] allData)
{
 var tasksData = new List<TaskData>();
 foreach (var task in allData)
 {
 string[] lineData = task.Split(',');
 tasksData.Add(new TaskData()
 {
 Name = lineData[0],
 // Before Enum: Status = lineData[1]
 Status = (Status)int.Parse(lineData[1].ToString())
 });
 };
 return tasksData;
}

Also your filter methods can then work as one-liners with lambda. I would be tempted here to split property Status into Status and StatusReadable. Make Status an Enum for ease of filtering.

public List<TaskData> ShowUnCompletedFromList(List<TaskData> allData)
{
 return allData.Where(w => w.Status == "Not completed").ToList();
}

Enum solution (I changed the ParseTaskData above):

internal enum Status
{
 NotCompleted = 0,
 Completed = 1
}

And TaskData then becomes:

internal class TaskData
{
 public string Name { get; set; }
 public Status Status { get; set; }
 public string StatusDescription
 {
 get
 {
 if (Status == Status.NotCompleted)
 {
 return "Not completed";
 }
 else if (Status == Status.Completed)
 {
 return "Completed";
 }
 else
 {
 return "Unknown";
 }
 }
 }
}

Filtering is now based on Enum and not magic strings:

public List<TaskData> ShowUnCompletedFromList(List<TaskData> allData)
{
 // With Magic string: return allData.Where(w => w.Status == "Not completed").ToList();
 return allData.Where(w => w.Status == Status.NotCompleted).ToList();
}

Try it out with Enum solution:

// Parse all tasks to List.
var tasksData = ParseTaskData(allData);
// Show items in List<>.
Console.WriteLine(">> All tasks:");
foreach (var task in tasksData)
{
 Console.WriteLine($"Task: {task.Name} is {task.StatusDescription}.");
}
Console.WriteLine(">> Uncompleted tasks:");
foreach (var uncompletedTask in filterData.ShowUnCompletedFromList(tasksData))
{
 Console.WriteLine($"Task: {uncompletedTask.Name} is {uncompletedTask.StatusDescription}.");
}
// Actually no need for a filter method, this one-liner does it the same as above.
Console.WriteLine(">> Uncompleted tasks short:");
filterData.ShowUnCompletedFromList(tasksData).ForEach(e => Console.WriteLine($"Task: {e.Name} is {e.StatusDescription}."));

• AMJ: Hi, thanks for reviewing, can your explain more about Make Status an Enum for ease of filtering how enum is easier to filter.
• Iztoksson: @AMJ I'm on my phone, I'll get back to you regarding first comment when I get to a PC. Regarding your second comment - yes, if you would need to print the data often or perhaps 'prettify' it etc. then go for a separate method. The Main method will be cleaner and you would promote code reusage (DRY).
• Iztoksson: @AMJ I changed to code to implement an Enum. Let me know if you have more questions.
• AMJ: Wow thanks a lot, using enum really was a game changer made the code easier to understand! Also I used Task.Name instead of Task[0] which look better as well. Is it possible to post my reviewed code here ? i dont know if the rules allows that.
• Iztoksson: @AMJ I'm not sure about the rules for posting modified code. If you keep the original question as-is and edit your solution below that, it would probably be ok.