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

Kevin Del Castillo Ramirez (JS Challenges) #33

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

quebin31
Copy link

@quebin31 quebin31 commented Oct 16, 2021

I noticed that there are some incorrect unit tests for ownPower and digitSum. For the former the last test is failing (i.e. ownPower(21, 12)) because 21 ** 21 is larger than Number.MAX_SAFE_INTEGER this already creates an overflow, if you don't use a BigInt from the beginning final sum is obviously going to be bigger than Number.MAX_SAFE_INTEGER, and for the latter something similar was happening, take for example the factorial of 42, this number is also going to be greater than Number.MAX_SAFE_INTEGER using number here is incorrect because it's going to produce an overflow.

Python interpreter:

>>> MAX_SAFE_INTEGER = 2 ** 53 - 1
>>> 21 ** 21 > MAX_SAFE_INTEGER
True
>>> import math
>>> math.factorial(42) > MAX_SAFE_INTEGER
True

To prove these points I included a Python3 script (challenge.py) with implementations to those two functions, but with the difference that Python3 supports arbitrarily large numbers out of the box, if you run this script you'll see the correct values.

@@ -41,7 +57,13 @@ Invoking "circularArray(2)" should return "["Island", "Japan", "Israel", "German
const COUNTRY_NAMES = ["Germany", "Norway", "Island", "Japan", "Israel"];

const circularArray = (index) => {
// YOUR CODE HERE...
if (index < 0) throw "Index cannot be negative";
Copy link
Contributor

Choose a reason for hiding this comment

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

use single quotes

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