Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update buy ins #72

Merged
merged 14 commits into from
Oct 14, 2024
Merged

Update buy ins #72

merged 14 commits into from
Oct 14, 2024

Conversation

poojakedia
Copy link
Collaborator

No description provided.

@poojakedia poojakedia changed the base branch from main to dev October 6, 2024 17:11
@poojakedia poojakedia linked an issue Oct 6, 2024 that may be closed by this pull request
total_points: 15,
buy_ins: [
{ prize_id: 'prize1', buy_in: 5 },
{ prize_id: 'prize2', buy_in: 10 },
Copy link
Member

Choose a reason for hiding this comment

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

wouldn't this be 10 + 5 = 15? if the user has 15 points, then they should be able to spend 15 points. perhaps just do one point over

@kevinmonisit
Copy link
Member

kevinmonisit commented Oct 13, 2024

Hi @poojakedia, there are few things you could do to improve this. Right now, I will do the changes for you so no worries.

Instead of nesting if statements, you could use something called Guard Clauses to improve the readability of your code.
See here: https://refactoring.guru/replace-nested-conditional-with-guard-clauses

Because the if statements were nested, it looks like you forgot to return an error when the lengths of buy ins and requested buy ins are not equal. When testing on Postman, I got a 502 Bad Gateway error instead of a more productive error message when the buy_in array had differing array lengths.

Those are the things I see at first glance, but thank you so much for you work! We are really happy for your contributions and I think the hundreds of hackers that will be using your work will appreciate it as well :) !!!

@ethxng ethxng merged commit 9793f07 into dev Oct 14, 2024
2 checks passed
@kevinmonisit kevinmonisit deleted the update-buy-ins branch October 14, 2024 16:18
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.

Feat: new endpoint called /update-buy-ins
4 participants