-
-
Notifications
You must be signed in to change notification settings - Fork 9
add bookstore problem #92
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
Conversation
config.json
Outdated
| "uuid": "590c6355-dbb4-44a0-a2d7-98490055c7e9", | ||
| "practices": [], | ||
| "prerequisites": [], | ||
| "difficulty": 3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you say this is an easy exercise? The "global" average across tracks is 6.4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would call this a tricky one, let's make this a 5, shall we?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still waiting for this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked at the solution provided here. It is different from the one I built for other tracks but it passes the tests. This solution is way more compact than mine (which is good) but I am having problem following the logic. I think this is definitively a difficult problem (at least for me(, the instructions make it look easy but it isn't. I would vote for a 9 but would be okay if it is at least a 5.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rmonnet
My solution based on an observation for this particular 5 book series problem:
- There is a steep discount from 3 series to 4 series, 10% to 20%, no other tier jump offer this much benefit
- In case where we can build 5-3 set to get discount of 25% on 5 and 10% on 3, we can and should always rearrange them to 4-4 set to get discount of 20% on 8 books. We get better discount without any catch, 0.25 * 5 + 0.1 * 3 = 1.55 < 0.2 * 8 = 1.6
The rest are just implementation detail. I count book count for each series, sort them, so that we buy good value set first. Then I try to upgrade 3-set to 4-set for the cost of downgrading 5-set to 4-set, which is a net gain as we observed earlier.
rmonnet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few comments on style.
rmonnet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a comment on the get_discount() nested procedure and an opinion on the exercise difficulty (a 9 for me).
| price := base_price * f32(m) * f32(x) | ||
| discount: f32 | ||
| switch m { | ||
| get_discount :: proc(book: int) -> f32 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for not being clear in my previous comment. I would have defined a procedure (exactly like you did) but I would also have move the definition outside of the body of calculate() (tmake it a op-level in example.odin). What this would do is make the body of calculate() easier to read (in my opinion).
I know we can nest procedures in Odin but the reason to do that would be to hide the nested procedure from the users of the package, but at the cost of making the procedure that includes the nested procedure harder to read (again my opinion). To me, this is a little bit like the do statement, some additional feature that doesn't bring anything to the language.
However, this is a matter of style so don't feel like you have to change the code if this looks cleaner to you. I am just thinking aloud.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comment. I want to communicate to learner that in Odin we can declare nested functions like that for utilities that are only relevant in that context and never meant to be used outside. That reduce cognitive load. Do you think that is a good practice?
No description provided.