Finished project

[davetn657]

No description provided.

[chrisjamiecarter]

Hey @davetn657 👋,

Excellent work on your Shifts Logger project submission 🎉!

I have performed a peer review. Review/ignore any comments as you wish.

---

🟢 Requirements

⭐ You have fulfilled all of the project requirements!

• WebAPI records worker shifts
• Two applications created (WebAPI + Console UI)
• Validation occurs in UI App
• API controller is lean
• Code-First Approach with EF Core migrations
• Shift duration calculated from start and end times

---

I will go ahead and mark as approved, keep up the excellent work on the next projects! 😊

Best regards,
@chrisjamiecarter 👍

[chrisjamiecarter]

🟠 Namespaces

💡 As per the academy's code-conventions, use file-scoped namespaces.

➡️ From C# 10, we are able to remove the block/braces and save on level of indentation, optimising space and improving readability.

Suggested change

	namespace ShiftsLogger.davetn657.Controllers
	{
	internal class Validation
	{
	public Validation()
	{
	}
	internal DateTime? IsValidTime(string time)
	{
	if (DateTime.TryParse(time, out DateTime convertedTime)) return convertedTime;
	return null;
	}
	internal DateTime? IsValidTime(DateTime? startTime, DateTime? endTime)
	{
	if (endTime < startTime) return null;
	return endTime;
	}
	}
	}
	namespace ShiftsLogger.davetn657.Controllers;
	internal class Validation
	{
	public Validation()
	{
	}
	internal DateTime? IsValidTime(string time)
	{
	if (DateTime.TryParse(time, out DateTime convertedTime)) return convertedTime;
	return null;
	}
	internal DateTime? IsValidTime(DateTime? startTime, DateTime? endTime)
	{
	if (endTime < startTime) return null;
	return endTime;
	}
	}

[chrisjamiecarter]

🟠 Static Class/Methods

💡 These should be static as they do not require an instance.

Suggested change

	internal class Validation
	{
	public Validation()
	{
	}
	internal DateTime? IsValidTime(string time)
	{
	if (DateTime.TryParse(time, out DateTime convertedTime)) return convertedTime;
	return null;
	}
	internal DateTime? IsValidTime(DateTime? startTime, DateTime? endTime)
	{
	if (endTime < startTime) return null;
	return endTime;
	}
	}
	internal static class Validation
	{
	internal static DateTime? IsValidTime(string time)
	{
	if (DateTime.TryParse(time, out DateTime convertedTime)) return convertedTime;
	return null;
	}
	internal static DateTime? IsValidTime(DateTime? startTime, DateTime? endTime)
	{
	if (endTime < startTime) return null;
	return endTime;
	}
	}

[chrisjamiecarter]

🟡 Inconsistent Property Naming

⚠️ Property id should be Id to follow PascalCase convention for public API members. Even with [JsonPropertyName("id")], the C# property should follow naming conventions:

Suggested change

	public int id { get; set; }
	public int Id { get; set; }

[chrisjamiecarter]

🟡 Fire-and-Forget Async Call

⚠️ EditShift is an async method by called without await.

Suggested change

	EditShift(shifts.UserShifts[i]);
	await EditShift(shifts.UserShifts[i]);

[chrisjamiecarter]

🟡 Magic String

⚠️ The string "r" is used as a magic value for return navigation. Extract to a named constant:

private const string ReturnOption = "r";
// Then use: if (name.ToLower() == ReturnOption)

[chrisjamiecarter]

🔵 Misleading Variable Name

ℹ️ Variable logNames suggests a collection of names but is actually a List<Shift>. Consider allShifts or shifts.

[chrisjamiecarter]

🔵 Copy-Paste Variable Name

ℹ️ Variable savedFlight appears to be a leftover from another project. Should be savedShift:

var savedShift = _dbContext.Shifts.Find(id);
return savedShift;

[chrisjamiecarter]

🟢 Good Use of Spectre.Console

⭐ Excellent Polished CLI experience with FigletText for titles and nice formatting!

[chrisjamiecarter]

🟢 Good Service Implementation

⭐ Good use of dependency injection and the service pattern with interface ILoggingService. This makes the code testable and follows SOLID principles!

[chrisjamiecarter]

🟢 Lean API Controller

⭐ The controller follows best practices - it's thin and delegates to the service layer. Well done!