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

Adding arithmetic computation: Power. #6

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

Conversation

GBT3101
Copy link

@GBT3101 GBT3101 commented Aug 10, 2020

Hey Ronen, I wish to support the meta-typing effort by adding a new arithmetic computation!

README.md Outdated
@@ -39,6 +39,7 @@ $ yarn test
- [Subtract](src/subtract/index.d.ts) - subtracts two numbers.
- [Multiply](src/multiply/index.d.ts) - multiplies two numbers.
- [Divide](src/divide/index.d.ts) - divides two numbers.
- [Power](src/power/index.d.ts) - computes A power B.
Copy link
Owner

Choose a reason for hiding this comment

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

How about computes the given base taken to the power of the given exponent?

@@ -0,0 +1,32 @@
import { Multiply, Dec, Cast } from '..';

// Power two numbers: https://lodash.com/docs/4.17.15#power.
Copy link
Owner

Choose a reason for hiding this comment

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

I think this can be the same description as in the README.md. Also, this URL doesn't lead anywhere, how about we use https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/pow instead?

// This type uses recursive (and not officially supported) type alias, see more:
// https://github.com/microsoft/TypeScript/issues/26223#issuecomment-513187373.
export type Power<
// The first number in a multiplication.
Copy link
Owner

Choose a reason for hiding this comment

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

I think multiplication is a mistake, how about The base number?

// https://github.com/microsoft/TypeScript/issues/26223#issuecomment-513187373.
export type Power<
// The first number in a multiplication.
A extends number,
Copy link
Owner

Choose a reason for hiding this comment

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

I think this can be B (short for base).

Copy link
Author

Choose a reason for hiding this comment

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

I think it breaks the consistency of other arithmetic computations in the project where A is always the first number of the evaluation (like in normal calculators)

export type Power<
// The first number in a multiplication.
A extends number,
// The second number in a multiplication.
Copy link
Owner

Choose a reason for hiding this comment

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

I think this is a mistake too, how about The exponent used to raise the base?

// The first number in a multiplication.
A extends number,
// The second number in a multiplication.
B extends number
Copy link
Owner

Choose a reason for hiding this comment

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

I think this can be E (short for exponent).

Copy link
Owner

@ronami ronami left a comment

Choose a reason for hiding this comment

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

Thanks for the PR 🏆, please see my comments 🙏

@GBT3101
Copy link
Author

GBT3101 commented Aug 27, 2020

@ronami I made the needed changes :)

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