Skip to content

smellbow first commit - #769

Closed
Smellbow wants to merge 1 commit into
the-csharp-academy:mainfrom
Smellbow:main
Closed

smellbow first commit#769
Smellbow wants to merge 1 commit into
the-csharp-academy:mainfrom
Smellbow:main

Conversation

@Smellbow

Copy link
Copy Markdown

Ok my first foray into c# and pull requests so apologies now if im doing something wrong!

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

@Smellbow Project approved! 😄✅ It's a shame you didn't complete the challenges this time. 😟 It's always a good use of your time. But nothing stops you from revisiting the project later. 😁 By the way the UI/UX is super nice with Spectre Console. I love the progress bar in the history 📊

🥋And congratulations! You got your green belt!! 🟩🟩

Feedback
🔍️ I highly recommend you learn about the DRY Principle (Don't Repeat Yourself) . There's a fair bit of repetition in the operations methods. You could have a single method for all operations, using variables for each operation.

🔍️A couple of smaller style points: Random could be shared instead of constructing a new instance in every Play...()

🔍Some comments such as // Return the score to the caller explain what the code already makes clear; comments are most valuable when they explain why something is being done. You can read our code conventions here🤓.

👪Oh and if possible join our Discord Community and add your username to your profile so you can be part of the community as well. I couldn't find the user "Smellbow" in our Discord😁

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

Overall great 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