Skip to content

Challenges completed - #32

Closed
FeedMyData wants to merge 2 commits into
the-csharp-academy:mainfrom
FeedMyData:main
Closed

FeedMyData wants to merge 2 commits into
the-csharp-academy:mainfrom
FeedMyData:main

Conversation

@FeedMyData

Copy link
Copy Markdown

(except the AI voices commands)

@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.

@TheCSharpAcademy TheCSharpAcademy 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.

@FeedMyData Thanks for submitting and apologies for the delay. Before we can review we need you to fix this issue:

⚠️ The project contains both Calculator and CalculatorLibrary, and Calculator.csproj correctly contains a ProjectReference to the library. However, there is no solution file (.sln) containing the two projects so it doesn't build.

Please rectify this and commit the changes when it's ready. No need to create a new pull-request😁

@TheCSharpAcademy TheCSharpAcademy self-assigned this Sep 7, 2026
@FeedMyData

Copy link
Copy Markdown
Author

Thanks for the feedback and no need to apologize. You are helping us on our freetime, I am the one who should thank you and apologize for the slow answer (I was on vacation).

@TheCSharpAcademy

Copy link
Copy Markdown

@FeedMyData Project approved! 😄✅And congratulations for completing some of the challenges. That's how you improve!! 👏👏 And you even cleaned up the original code from the tutorial, which is quite messy. 💪

Feedback
🔍️ One thing I'd improve is encapsulation. calculator.history and useCount are public fields. Prefer properties so you expose history as read-only where possible, and provide methods such as ClearHistory() instead of letting Program manipulate the calculator's internal collection directly. That would be a cleaner approach.

🔍One edge case to revisit is history logging. DoOperation() adds an OperationLog even when an operation produces NaN, such as division by zero or the square root of a negative number. It would be cleaner to only add successful calculations to the history

🗺️ I noticed that the only course you've completed so far is Introduction to Object-Oriented Programming. I'd highly recommend taking our LINQ Fundamentals course next. LINQ is one of the most important tools in modern C# and becoming comfortable with it will make your code much cleaner and more expressive.

☕If you like our roadmap, please consider buying us a coffee. We appreciate your help 🙂

Overall good work!✋🏻Looking forward to seeing your next projects!

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