-
-
Notifications
You must be signed in to change notification settings - Fork 556
pythagorean-triplet: implement canonical-data.json #1332
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
pythagorean-triplet: implement canonical-data.json #1332
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 looked at descriptions and have no objections. I cannot think of any new cases to add, and I have no requests to remove any proposed case. For those who request to remove any cases I advise that at least one case of each of the following be kept:
- one triplet
- zero triplets
> 1
triplets
Verifier says all proposed cases have the correct expectations.
require 'json'
require_relative '../../verify'
json = JSON.parse(File.read(File.join(__dir__, 'canonical-data.json')))
verify(json['cases'].map { |c| c.merge('expected' => c['expected'].sort) }, property: 'tripletsWithSum') { |i, _|
n = i[?n]
triplets = []
(1...(n / 3)).each { |a|
# c = n - a - b
# a^2 + b^2 = c^2
# a^2 + b^2 = n^2 - 2an - 2bn + a^2 + 2ab + b^2
# 2bn - 2ab = n^2 - 2an
# 2b(n - a) = n(n-2a)
# b = n(n-2a) / 2(n-a)
# b = (n(n-a) - an) / 2(n-a)
b = n / 2 - a * n / (2 * (n - a))
break if a >= b
c = n - a - b
triplets << [a, b, c] if a * a + b * b == c * c
}
triplets
}
Note the need to sort the expected output, simply because I understand the expected output has no ordering. It would be convenient for humans if the list presented in JSON were sorted, but I'm Approving (in the GitHub sense) as is anyway.
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'm a bit unclear on how this resolves #1211 as it this does not modify description.md, but other than that, it looks fine to me. |
As there are 3 approvals already, I will go ahead and squash-merge this. Thank you everyone for your input. @petertseng, note that I have indeed sorted the expected output. @sdublish You are right, I have removed the link from the PR description so #1211 will remain open. |
Resolves #579
This data set was generated using this one-off script, for those who are curious about where the cases came from, with a runtime of ~21.5 seconds on my machine (primarily due to the test case for a large
n
).I welcome additional test suggestions or suggestions for modification/removal of currently present cases or case format.