-
Notifications
You must be signed in to change notification settings - Fork 1
Update todo/24 update todo #25
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
base: master
Are you sure you want to change the base?
Changes from all commits
c872dfb
5d7c6db
9f9589c
b75304d
2f07ce3
3d338b9
876b1fe
d851887
88d0cac
479f610
0f1c464
9cf2a9a
936a3c2
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 |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ public enum TODOMENU | |
| View, | ||
| MarkAsDone, | ||
| ListAll, | ||
| Update, | ||
| Quit | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| namespace EasyList | ||
| { | ||
| public enum TodoUpdate | ||
| { | ||
| Label, | ||
| Description, | ||
| Priority, | ||
| DueDate | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,12 @@ | ||
| namespace EasyList | ||
| using EasyList.Factories; | ||
| using EasyList.Interfaces; | ||
|
|
||
| namespace EasyList | ||
| { | ||
| class Program | ||
| { | ||
| public static ITodoService TodoService => Factory.CreateTodoServiceDB(); | ||
|
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. Correct naming convention.
Contributor
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. This is a field and not a Property so _todoService ? @Anequit
Member
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. It is actually a property. This is shorthand for a read-only property.
Contributor
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. Why is the service a static instance and inside program.cs if its not being used here?
Contributor
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.
So that is becomes very easy to identify where we declared it at one place.
Contributor
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.
@valincius I meant before changing it. As you suggested last time i changed it to a read only property here
Contributor
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.
What do you mean with "identify"? I have mentioned this before. You have set up a dependency inversion system with the factories, i would stick with that. You should first of all use encapsulation which is part of the OOP principles. Secondly there is no point in making a class non-static just to make it static afterwards. Also always try to keep Program.cs as clean as possible, it is only supposed to be used as the starting point of your application.
Contributor
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. Makes sense |
||
|
|
||
| public static void Main(string[] args) | ||
| { | ||
| if(args.Length > 1) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,138 +1,100 @@ | ||
| using EasyList.DataModels; | ||
| using EasyList.Enums; | ||
| using EasyList.Factories; | ||
| using EasyList.Interfaces; | ||
| using Sharprompt; | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
|
|
||
| namespace EasyList | ||
| { | ||
| public class TodoMenu | ||
| { | ||
| private static IEnumerable<string> ValidateAdd(Dictionary<string, string> input) | ||
| { | ||
| if (string.IsNullOrWhiteSpace(input["label"])) | ||
| { | ||
| yield return "Label cannot be empty"; | ||
| } | ||
| if (DateTimeOffset.TryParse(input["duedate"], out DateTimeOffset tempDate)) | ||
| { | ||
| if (tempDate < DateTime.UtcNow) | ||
| { | ||
| yield return "Due date cannot be in the past"; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private static bool Validate(TODOMENU command, Dictionary<string, string> input) | ||
| { | ||
| var errors = command switch { | ||
| TODOMENU.Add => ValidateAdd(input), | ||
| // register the rest of the options that need to be validated here | ||
| _ => throw new InvalidOperationException($"No validator exists for {command}"), | ||
| }; | ||
|
|
||
| if (errors.Any()) | ||
| { | ||
| Console.WriteLine("The input has the following errors:"); | ||
| foreach (var error in errors) | ||
| { | ||
| Console.WriteLine("\t"+error); | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| public static void Run() | ||
| { | ||
| ITodoService _todoService = Factory.CreateTodoServiceDB(); | ||
| while(true) | ||
| var _todoValidate = new Validate(); | ||
|
Contributor
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. The class is Todo specific and should be called TodoValidator, i think a name for the field should be the same name as the class |
||
| while (true) | ||
| { | ||
| var action = Prompt.Select<TODOMENU>("Welcome to EasyList!"); | ||
| //Refactor this such that adding new should not modify this input layer | ||
| switch (action) | ||
|
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. And if statement would be a bit cleaner. Especially when you have logic in every case.
Contributor
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. What do you think about this ? @Anequit |
||
| { | ||
|
|
||
| case TODOMENU.Add: | ||
| var inputAdd = Prompt.Input<string>("Enter TODO "); | ||
| var parsedAdd = ParseAdd.Parse(inputAdd?.Split() ?? Array.Empty<string>()); | ||
|
|
||
| if (Validate(action, parsedAdd)) | ||
| { | ||
| var newTodo = new Todo() | ||
| var inputAdd = Prompt.Input<string>("Enter TODO "); | ||
| var parsedAdd = ParseAdd.Parse(inputAdd?.Split() ?? Array.Empty<string>()); | ||
|
|
||
| if (_todoValidate.IsAddValid(parsedAdd)) | ||
| { | ||
| Label = parsedAdd["label"], | ||
| Description = parsedAdd["description"], | ||
| DueDate = DateTimeOffset.TryParse(parsedAdd["duedate"], out DateTimeOffset tempDate) ? tempDate : null, | ||
| Priority = Enum.Parse<TodoPriority>(parsedAdd["priority"]) | ||
| }; | ||
| _todoService.AddTodo(newTodo); | ||
| var newTodo = new Todo() | ||
| { | ||
| Label = parsedAdd["label"], | ||
| Description = parsedAdd["description"], | ||
| DueDate = DateTimeOffset.TryParse(parsedAdd["duedate"], out DateTimeOffset tempDate) ? tempDate : null, | ||
| Priority = Enum.Parse<TodoPriority>(parsedAdd["priority"]) | ||
| }; | ||
| Program.TodoService.AddTodo(newTodo); | ||
|
Contributor
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. TodoService should be in this class instead |
||
| } | ||
| break; | ||
| } | ||
|
|
||
| break; | ||
|
|
||
| case TODOMENU.Delete: | ||
| var inputDelete = Prompt.Input<string>("Enter TODO ID(s) "); | ||
| foreach (var item in inputDelete.Split()) | ||
| var inputDelete = Prompt.Input<string>("Enter TODO ID(s) ").Split(); | ||
|
|
||
| if (_todoValidate.IsDeleteValid(inputDelete,out List<Todo> todoDeleteList)) | ||
| { | ||
| var todoItem = _todoService.GetTodoByID(int.Parse(item)); | ||
| if(todoItem != null) | ||
| foreach (var todoDelete in todoDeleteList) | ||
| { | ||
| Console.WriteLine($"Deleted: {todoItem.Label}"); | ||
| _todoService.DeleteTodo(todoItem); | ||
| } | ||
| else | ||
| { | ||
| Console.WriteLine($"Todo Id: {item} Not Found."); | ||
| Console.WriteLine("Try Again."); | ||
| } | ||
|
|
||
| Program.TodoService.DeleteTodo(todoDelete); | ||
| } | ||
| } | ||
| break; | ||
|
|
||
| case TODOMENU.View: | ||
| var inputView = Prompt.Input<int>("Enter TODO ID "); | ||
| var todo = _todoService.GetTodoByID(inputView); | ||
| if(todo != null) | ||
| { | ||
| _todoService.DisplayTodo(todo); | ||
| } | ||
| else | ||
| { | ||
| Console.WriteLine($"Todo Id: {inputView} Not Found."); | ||
| var inputView = Prompt.Input<string>("Enter TODO ID "); | ||
|
|
||
| if (_todoValidate.IsReadValid(inputView, out Todo validTodo)) | ||
| { | ||
| Program.TodoService.DisplayTodo(validTodo); | ||
| } | ||
| break; | ||
| } | ||
| break; | ||
|
|
||
| case TODOMENU.MarkAsDone: | ||
| var inputDone = Prompt.Input<string>("Enter TODO ID(s) "); | ||
| foreach (var item in inputDone.Split()) | ||
| { | ||
| var todoItem = _todoService.GetTodoByID(int.Parse(item)); | ||
| if(todoItem != null) | ||
| var inputDone = Prompt.Input<string>("Enter TODO ID(s) ").Split(); | ||
|
|
||
| if (_todoValidate.CanMarkAsDone(inputDone, out List<Todo> doneTodoList)) | ||
| { | ||
| Console.WriteLine($"Completed:{todoItem.Label}."); | ||
| _todoService.MarkTodoAsDone(todoItem); | ||
| foreach (var doneTodo in doneTodoList) | ||
| { | ||
| Program.TodoService.MarkTodoAsDone(doneTodo); | ||
| } | ||
| } | ||
| else | ||
| break; | ||
| } | ||
| case TODOMENU.Update: | ||
| { | ||
| var inputUpdateAction = Prompt.Select<TodoUpdate>("Select Update Action"); | ||
| var inputUpdate = Prompt.Input<string>("Enter the ID to Update"); | ||
|
|
||
| if (_todoValidate.IsReadValid(inputUpdate, out Todo validTodo)) | ||
| { | ||
| Console.WriteLine($"Todo Id: {item} Not Found."); | ||
| Console.WriteLine("Try Again."); | ||
| Program.TodoService.UpdateTodo(validTodo, inputUpdateAction); | ||
| } | ||
| break; | ||
| } | ||
| break; | ||
|
|
||
| case TODOMENU.ListAll: | ||
| var inputList = Prompt.Select<TodoOrder>("Select List Order: ", defaultValue: TodoOrder.CreateDate); | ||
| _todoService.DisplayAllTodo(inputList); | ||
| break; | ||
| { | ||
| var inputList = Prompt.Select<TodoOrder>("Select List Order: ", defaultValue: TodoOrder.CreateDate); | ||
| Program.TodoService.DisplayAllTodo(inputList); | ||
| break; | ||
| } | ||
| case TODOMENU.Quit: | ||
| Console.WriteLine("Exiting..."); | ||
| Environment.Exit(Environment.ExitCode = 0); | ||
| break; | ||
| { | ||
| Console.WriteLine("Exiting..."); | ||
| Environment.Exit(Environment.ExitCode = 0); | ||
|
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. Environment.Exit(0); does the same thing |
||
| break; | ||
| } | ||
| } | ||
|
|
||
| Console.ReadLine(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
| using EasyList.Enums; | ||
| using EasyList.Factories; | ||
| using EasyList.Interfaces; | ||
| using Sharprompt; | ||
|
|
||
| namespace EasyList | ||
| { | ||
|
|
@@ -50,6 +51,58 @@ public void DisplayAllTodo(TodoOrder todoOrder) | |
| .Write(Format.MarkDown); | ||
| } | ||
|
|
||
|
|
||
| public void UpdateTodo(Todo todo, TodoUpdate command) | ||
| { | ||
| var _todoValidate = new Validate(); | ||
| switch (command) | ||
|
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. If statement would be better here too. |
||
| { | ||
| case TodoUpdate.Label: | ||
| { | ||
| Console.WriteLine("Enter the New Label: "); | ||
| string? newLabel = Console.ReadLine(); | ||
| if (_todoValidate.IsLabelValid(newLabel)) | ||
| { | ||
| todo.Label = newLabel; | ||
| _todoRepository.UpdateTodo(todo); | ||
|
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. This line is being repeated in every case. |
||
| Console.WriteLine("Label Updated Sucessfully."); | ||
| } | ||
| break; | ||
| } | ||
|
|
||
| case TodoUpdate.Description: | ||
| { | ||
| Console.WriteLine("Enter the New Description: "); | ||
| string? newDescription = Console.ReadLine(); | ||
| if (_todoValidate.IsUpdateDescriptionValid(newDescription)) | ||
| { | ||
| todo.Description = newDescription; | ||
| _todoRepository.UpdateTodo(todo); | ||
| Console.WriteLine("Description Updated Sucessfully."); | ||
| } | ||
| break; | ||
| } | ||
|
|
||
| case TodoUpdate.Priority: | ||
| { | ||
| todo.Priority = Prompt.Select<TodoPriority>("Select new Priority."); ; | ||
|
Contributor
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. service should not interact with the user, thats a job for the menu |
||
| _todoRepository.UpdateTodo(todo); | ||
| break; | ||
| } | ||
|
|
||
| case TodoUpdate.DueDate: | ||
| { | ||
| Console.WriteLine("Enter the New Due Date: "); | ||
| string? newDueDate = Console.ReadLine(); | ||
| if(_todoValidate.IsDueDateValid(newDueDate)) | ||
| { | ||
|
|
||
| todo.DueDate = DateTimeOffset.Parse(newDueDate!); | ||
| _todoRepository.UpdateTodo(todo); | ||
| Console.WriteLine("Due Date Updated Sucessfully."); | ||
| } | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||

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.
How did this get committed? Was the code not debugged?
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.
Yup, Maybe Expecting only a silly string change I didn't check after this commit.