-
Notifications
You must be signed in to change notification settings - Fork 4
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
initial problem generator module #39
Conversation
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 recommend making some changes, but they aren't really required. 👍
src/lib/ProblemGenerator.ts
Outdated
let num = this.generateNumber(digits, true); | ||
result.push(num); | ||
|
||
if (result.reduce((sum, n) => sum + n, 0) <= 0) { | ||
result.pop(); | ||
} |
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 suggest instead replacing all these checks by
- Keeping a running sum separate to
result
- Passing
-theRunningSum
togenerateNumber
as something likefurthestNegativeAllowed
- Rejecting negation if the number it generates is less than
furthestNegativeAllowed
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.e., keeping this method O(n) rather than unbounded if you get mega unlucky.
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.
let me know if you see any issues with the updated one.
src/lib/ProblemGenerator.ts
Outdated
return [num1, num2]; | ||
} | ||
|
||
divide(digits1: number, digits2: number): number[] { |
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.
Perhaps the result of this, rather than number[]
, should be some sort of ProblemSet
class with an entries: number[]
field and a result: number
field.
added class ProblemSet
No description provided.