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

refactor: add number util #408

Merged
merged 7 commits into from
May 20, 2024
Merged

refactor: add number util #408

merged 7 commits into from
May 20, 2024

Conversation

lissavxo
Copy link
Collaborator

Related to #405

Description

Added handler with BigNumber logic in utils

Test plan

yarn build && yarn dev

@lissavxo lissavxo requested review from chedieck and Klakurka May 17, 2024 06:46
chedieck
chedieck previously approved these changes May 17, 2024
Copy link
Member

@Klakurka Klakurka left a comment

Choose a reason for hiding this comment

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

How about adding some appropriate tests?

@lissavxo lissavxo force-pushed the feat/add-number-util branch from 97dee4a to aa9bb7b Compare May 20, 2024 15:45
expect(zero.isEqualTo(0)).toBe(true);
});

it('resolveNumber should convert various types to BigNumber', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's tests floats too (42.0, 42.5) and string floats ('42.0', '42.5'). Also, they shouldn't be in the same tests

});

it('isLessThanZero should correctly identify values less than zero', () => {
expect(isLessThanZero(-1)).toBe(false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

isLessThanZero(-1) returns false? why?

return new BigNumber(value);
};

export const isLessThanZero = (value: BigNumber.Value) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This function is implemented backwards. It's named isLessThanZero but it checks if zeroIsLessThan

@lissavxo lissavxo force-pushed the feat/add-number-util branch from 020c9b7 to a4a26f5 Compare May 20, 2024 21:02
@lissavxo lissavxo requested a review from chedieck May 20, 2024 21:05
@lissavxo lissavxo force-pushed the feat/add-number-util branch from 4eb35d3 to fe749fd Compare May 20, 2024 21:12
@Klakurka Klakurka merged commit 59d41cf into master May 20, 2024
1 check passed
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.

3 participants