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

Remove dependency on bignumber #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Remove dependency on bignumber #6

wants to merge 1 commit into from

Conversation

axic
Copy link
Member

@axic axic commented Apr 13, 2016

Fixes #3

@axic
Copy link
Member Author

axic commented Apr 13, 2016

@wanderer @fanatid comments (and improvements) welcome

@coveralls
Copy link

Coverage Status

Coverage decreased (-2.04%) to 97.959% when pulling 6d8e640 on feature/no-bignum into 7247265 on master.

})

Units.units = rawUnits

// add leading zeroes
// FIXME: improve
function zeroes (n) {
Copy link
Member

Choose a reason for hiding this comment

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

you could replace this with return Array(n).fill(0).join('') but i don't know if that is more or less efficient

Copy link

Choose a reason for hiding this comment

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

you can preallocate big enough zero string and then just do zeros.slice(0, n)

Copy link

Choose a reason for hiding this comment

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

Copy link
Member Author

@axic axic Apr 14, 2016

Choose a reason for hiding this comment

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

you can preallocate big enough zero string and then just do zeros.slice(0, n)

I was thinking about this, zeroes would need to be either 31 zeroes (256bits) or the largest in units.json. Any quick way to find the largest unit in units.json?

or you can use https://www.npmjs.com/package/left-pad :)

I think I leave left-pad alone :)

Copy link

Choose a reason for hiding this comment

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

I was thinking about this, zeroes would need to be either 31 zeroes (256bits) or the largest in units.json. Any quick way to find the largest unit in units.json?

Sorry, not familiar with ethereum units.. maybe 256-characters length string will be enough? :)

@wanderer
Copy link
Member

so is this ready to merge?

@axic
Copy link
Member Author

axic commented Jun 9, 2016

Checked against more tests and found that on certain cases it leaves a trailing .

Need to check why.

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.

None yet

4 participants