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

Add utility functions and tests to signed integers #68

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

Conversation

chlenc
Copy link

@chlenc chlenc commented Jan 4, 2023

Type of change

  • Bug fix
  • New feature

Changes

The following changes have been made to I128 and I256:

  • added functions

fn from_u64(value: u64) -> I128
fn as_u64(self) -> u64
fn flip(self) -> Self
fn ge(self, other: Self) -> bool
fn le(self, other: Self) -> bool
fn zero() -> I128

  • A tests
  • Added check to test not to be failed

The following changes have been made to I8, I16, I32 and I64:

  • added functions

fn flip(self) -> Self
fn ge(self, other: Self) -> bool
fn le(self, other: Self) -> bool

  • A tests
  • Added check to test not to be failed

Notes

But my tests are still failing because of the problem I have mentioned before in this topic
https://forum.fuel.network/t/cannot-use-from-uint-in-the-i128-from-sway-libs/1255

@supiket supiket changed the title ✨ added some useful functions and tests to I128 Add utility functions and tests to I128 Jan 4, 2023
@supiket supiket changed the title Add utility functions and tests to I128 Add utility functions and tests to signed integers Jan 4, 2023
Copy link

@supiket supiket left a comment

Choose a reason for hiding this comment

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

These are my comments for this stage of the work, but since this PR is a work in progress, I will convert this open PR to draft. I also edited the title to reflect that these changes are implemented for all signed integer types. You can mark it ready for review (open) once functions for other types and tests for all the new functions are added.

underlying: Self::indent() - self.underlying,
}
}
}
Copy link

Choose a reason for hiding this comment

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

Suggested change
}
}

Copy link
Author

Choose a reason for hiding this comment

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

🫡

@@ -16,6 +16,8 @@ mod success {

let instance = Testi128::new(wallet, path_to_bin);

let _result = instance.main().call().await;
let params = TxParameters::new(Some(1), Some(10000000), None);
Copy link

Choose a reason for hiding this comment

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

Same suggestion for other instances of this, for readability purposes.

Suggested change
let params = TxParameters::new(Some(1), Some(10000000), None);
let params = TxParameters::new(Some(1), Some(10_000_000), None);

Comment on lines 20 to 21
let _result = instance.main().tx_params(params).call().await;
assert_eq!(_result.is_err(), false);
Copy link

Choose a reason for hiding this comment

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

The underscore is used for unused variables.
Same suggestion for other instances of this usage.

Suggested change
let _result = instance.main().tx_params(params).call().await;
assert_eq!(_result.is_err(), false);
let result = instance.main().tx_params(params).call().await.unwrap();
assert!(result.value);

@supiket supiket marked this pull request as draft January 4, 2023 10:28
@chlenc chlenc marked this pull request as ready for review January 4, 2023 20:59
@chlenc chlenc closed this Jan 4, 2023
@chlenc chlenc reopened this Jan 4, 2023
@bitzoic bitzoic added Improvement Enhancing a feature that already exists Lib: Signed Math labels Jan 9, 2023
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


alexey seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Enhancing a feature that already exists
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants