Finished guided project habit tracker#5
Conversation
…ion which is exciting
DSills735
left a comment
There was a problem hiding this comment.
Hello ✋ @Hacker-735 ,
I am here to perform your peer review, I apoligize for the delay. Feel free to work on the future roadmap projects while you are waiting on our review.
Anything with the 🔴 needs to be fixed and blocks approval. 🟠 Is not a required fix, but is my reccomendation for cleaner code. 🟢 Are things I found that were done particularly well.
🔴 Blocking approval
- Nothing! Good work.
🟠 Needs work
-UI is messy in some areas, but cleaner in others. Use Console.Clear() as much as possible. This really will clean up a UI in a console app.
-
I cannot type td if I mis input the date originally. Not a big deal, but definitely an annoyance if In am working with this app regularly.
-
I cannot dissaprove now because of this, but with SQL commands, it is standard to not use string interpolation. Interpolation in commands is where SQL injection comes from, and is a major security risk.
-This is not a requirement of this project, but if you get much bigger than this,code becomes hugely disorganized. Using good SOC practice early and often will help alot. (I believe this is a requirement for all projects moving forward.)
🟢 Good things
-UI is clean with options 3/4.
-Validations are good and accrately handle edge cases.
- Other than the SQL issues, code is written cleanly, I can easily understand the intentions of code. Once note I will make is consider CS8600. This is "just a warning", but it does have its place. You do properly handle it. How can we make this warning dissapear? No need to fix this, just tryiong to provoke thought for the future.
Thank you! Good work!
this is my first CRUD application which is exciting