Skip to content

Finished. - #43

Closed
abdukulov-talgat wants to merge 10 commits into
the-csharp-academy:mainfrom
abdukulov-talgat:main
Closed

abdukulov-talgat wants to merge 10 commits into
the-csharp-academy:mainfrom
abdukulov-talgat:main

Conversation

@abdukulov-talgat

Copy link
Copy Markdown

This time it's simpler.

@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

Copy link
Copy Markdown

@abdukulov-talgat Project approved! 😄✅Well done for completing the challenges! You followed the tutorials and modified the code very effectively for the challenges. You also cleaned up the messy code from the tutorial quite a lot. Well done! 👏👏

Feedback
🔍️One thing I'd improve is the number validation:

Regex.IsMatch(str, "^p$|\\d+")

This doesn't properly support inputs such as negative numbers or decimals, despite the calculator using double. double.TryParse would be a much better fit and would simplify the validation at the same time.

🔍Improve file error handling. Loading calculator-log.json is protected by a try/catch, but Save() isn't. A file permission or disk error could therefore crash the application when performing or clearing a calculation.

🗺️ I saw you completed the Introduction to Object-Oriented Programming, I hope you enjoyed it! Next I 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