Skip to content

Changes made - #294

Open
jcisco22 wants to merge 13 commits into
the-csharp-academy:mainfrom
jcisco22:main
Open

jcisco22 wants to merge 13 commits into
the-csharp-academy:mainfrom
jcisco22:main

Conversation

@jcisco22

Copy link
Copy Markdown

Duration recalculated on update, end time validated after start, README added.

@github-actions

Copy link
Copy Markdown

🚀 Project Submitted!

Thanks for submitting your project! I will review it soon.

📋 Please check the dashboard to make sure the project is marked as Pending.

⚠️ If it's not marked as pending, submit it manually using the URL for this PR.

@0lcm 0lcm self-assigned this Oct 26, 2026
@0lcm

0lcm commented Oct 26, 2026

Copy link
Copy Markdown

Hey @jcisco22, I was reviewing your project and I noticed that there's no way to exit the delete and update menus, and if you enter those menus while the database is empty the user can get soft locked in that menu. I noticed that in the update menu you included instructions to type 0 to return to the main menu, but that feature doesn't seem to be working. If you could please fix this I would love to continue reviewing your project! 😄
image_2026-10-26_191651753
image

@jcisco22

Copy link
Copy Markdown
Author

Will do

…n the user enters 0. Insert now uses ValidateEndTime instead of duplicating the check. Removed recursive menu call from GeTimeInput. Restored dd-MM-yyyy HH:mm format.
@jcisco22

Copy link
Copy Markdown
Author

Return to main menu feature now fixed.

@0lcm 0lcm left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey @jcisco22, I left a few comments on your code, mostly regarding some unused variables and statements, and just a couple things for keeping your code clean. If you could look over these comments when you have time and clean the code up a little I'd appreciate it! 🙃

Comment on lines +1 to +3
using Microsoft.Data.Sqlite;
using Dapper;
using Spectre.Console;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These using statements are never used and can be safely removed from this file


class Program1
{
static string connectionString = @"Data Source=habit-Tracker.db";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This variable is never used and can be safely removed

{
static string connectionString = @"Data Source=habit-Tracker.db";

static void Main(string[] args)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No arguments are used in this method so the optional string[] args parameter isn't necessary and can be removed


class CodingController
{
// CodingController.cs — database operations

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This comment is unnecessary and mostly adds clutter to the code. A good rule of thumb for something like this is that the class should be named something to where it's easy to understand the purpose of the class, that way you don't have to add a comment explaining what the class should do

{
// CodingController.cs — database operations

static string connectionString = new ConfigurationBuilder()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For a private static string the general naming convention would be to have a '_' in front of the name like _connectionString. It's not a big deal but it helps make your code cleaner and easier to refactor, plus it's usually just a good idea to follow naming conventions

}
}

public static void Update(int ID, DateTime startTime, DateTime endTime, string duration)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The parameter int ID doesn't follow camelCase conventions, it would make more sense to name it something like id

@@ -0,0 +1,15 @@
// CodingSession.cs — the model

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This comment isn't necessary and can be removed to keep clutter to a minimum

@@ -0,0 +1,115 @@
using System.Runtime.ConstrainedExecution;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This using statement isn't necessary and can be safely removed

Comment on lines +7 to +8
DateTime startTime;
DateTime endTime;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Both of these variables are never used after being declared, and can be removed from the code


return endTime;
}
public static int ValidateId(string Input)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This parameter doesn't match camelCase conventions, a better name would be something like input

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