Open
Conversation
…rea, and binary exponentiation functions
appgurueu
requested changes
Oct 3, 2024
Contributor
appgurueu
left a comment
appgurueu
left a comment
There was a problem hiding this comment.
- All the area calculation functions are algorithmically pretty uninteresting formulae and should thus be removed IMO. Same for the geometric mean, though that might be more useful. I'm slightly on the fence about the triangle inequality checking; it's probably fine either way.
- Please get rid of redundant
@functiondoc comments. - Please get rid of unrelated changes:
package.json,package-lock.json
| /** | ||
| * @function binaryExponent | ||
| * @description Returns the power of a number A raised to the power of B with binary exponentiation | ||
| * @summary Binary exponentiation calculatoes A^B in logarithmic time of B instead of linear time |
Contributor
There was a problem hiding this comment.
Suggested change
| * @summary Binary exponentiation calculatoes A^B in logarithmic time of B instead of linear time | |
| * @summary Binary exponentiation calculates A^B in time O(log B) instead of O(B) |
| * @function binaryExponent | ||
| * @description Returns the power of a number A raised to the power of B with binary exponentiation | ||
| * @summary Binary exponentiation calculatoes A^B in logarithmic time of B instead of linear time | ||
| * @param {Number} num - An array of two natural numbers, [A, B], where A^B will be solved |
Contributor
There was a problem hiding this comment.
Just take two numbers, base and exponent, as parameters instead.
| * @summary Binary exponentiation calculatoes A^B in logarithmic time of B instead of linear time | ||
| * @param {Number} num - An array of two natural numbers, [A, B], where A^B will be solved | ||
| * @return {number} A^B | ||
| * @see [Wikipedia](https://cp-algorithms.com/algebra/binary-exp.html) |
| throw new TypeError('Invalid Input') | ||
| } | ||
| for(let i=0; i < numbers.length; i++){ | ||
| if (numbers[i] < 0 || !Number.isInteger(numbers[i])) { |
Contributor
There was a problem hiding this comment.
The base can very well be negative. You could also support negative exponents but you don't have to.
| export const binaryExponent = (numbers: number[]): number => { | ||
| // raise errors upon invalid inputs | ||
| if (numbers.length != 2) { | ||
| throw new TypeError('Invalid Input') |
Contributor
There was a problem hiding this comment.
This becomes obsolete if you fix the signature.
| console.log(factors1) | ||
|
|
||
| // check if same factors | ||
| for (let i = 0; i <= factors0.length; i++) { |
Contributor
There was a problem hiding this comment.
Could be optimized e.g. by doing a sorted merge instead. This is O(nm) but could be O(n + m). Not really necessary since the factorization is more expensive though (which should be noted however).
| import { binaryExponent } from '../binary_exponentiation' | ||
|
|
||
| describe('Tests for HexArea', () => { | ||
| it('should be a function', () => { |
| @@ -0,0 +1,29 @@ | |||
| import { binaryExponent } from '../binary_exponentiation' | |||
|
|
|||
| describe('Tests for HexArea', () => { | |||
Contributor
There was a problem hiding this comment.
Suggested change
| describe('Tests for HexArea', () => { | |
| describe('HexArea', () => { |
| @@ -0,0 +1,25 @@ | |||
| import { coPrime } from '../co_prime' | |||
|
|
|||
| describe('Tests for CoPrime', () => { | |||
Contributor
There was a problem hiding this comment.
Suggested change
| describe('Tests for CoPrime', () => { | |
| describe('CoPrime', () => { |
| import { coPrime } from '../co_prime' | ||
|
|
||
| describe('Tests for CoPrime', () => { | ||
| it('should be a function', () => { |
Contributor
|
Please remove the coprime check: #250 was opened earlier and implements a strictly better algorithm, obsoleting this one. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Includes changes related to Issue: #10
Included new maths functions and tests to calculate the geometric mean, triangle inequality, coprime, binary exponentiation, and area of regular hexagon, pentagon, and octagon, plus the tests for each.