UInt extends BN #396
UInt extends BN #396
Conversation
| } | ||
|
|
||
| get bitLength (): UIntBitLength { | ||
| bitLength (): UIntBitLength { |
There was a problem hiding this comment.
Q: Do we actually use/rely on bitLength anywhere? (Or is is just there as a "could be useful"). The reason for asking, is that I for the life of me cannot remember why we have it.
| @@ -27,8 +27,6 @@ export default class Moment extends Base<Date> { | |||
| return value.raw; | |||
| } else if (value instanceof Date) { | |||
| return new Date(Math.ceil(value.getTime() / 1000) * 1000); | |||
| } else if (value instanceof UInt) { | |||
There was a problem hiding this comment.
In removing this, we don't seem to cater for BN construction anymore? Should we not have a isBn path as well?
There was a problem hiding this comment.
The return at the bottom of the function handles the BN case.
| @@ -147,6 +147,6 @@ export default class Compact extends Base<UInt | Moment> { | |||
| } | |||
|
|
|||
| toU8a (isBare?: boolean): Uint8Array { | |||
| return Compact.encodeU8a(this.raw.toBn(), this.bitLength); | |||
| return Compact.encodeU8a(this.raw.toBn(), this.bitLength()); | |||
There was a problem hiding this comment.
Ahh... ok, to my question above, here it is used. (I should have probably left a note in there somewhere as to why we have bitLength)
| @@ -40,7 +38,7 @@ export default class Moment extends Base<Date> { | |||
| ); | |||
| } | |||
|
|
|||
| get bitLength (): UIntBitLength { | |||
| bitLength (): UIntBitLength { | |||
There was a problem hiding this comment.
Is there a reason why these are now not getters? Additionally, I believe in U8aFixed we still have it as a getter?
There was a problem hiding this comment.
I would love to have bitLength everywhere as getter too, but on BN, bitLength is a method, so on Moment it needs to be a method too.
I'll change bitLenght on U8aFixed to method too, for consistency.
There was a problem hiding this comment.
Alternatively, @ts-ignore in Uint, but we stray away from BN
There was a problem hiding this comment.
No, rather keep it as a method, it is cleaner.
| } | ||
|
|
||
| toBn (): BN { | ||
| return this.raw; |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
related #161.